Solving potential thread collision issues upon Layout::update.

src/db/db/dbLayout.cc

@@ -1749,8 +1749,49 @@ Layout::end_top_cells () const
   return m_top_down_list.begin () + m_top_cells;
 }
 
+void
+Layout::start_changes ()
+{
+  tl::MutexLocker locker (&lock ());
+  ++m_invalid;
+}
+
+void
+Layout::end_changes ()
+{
+  tl::MutexLocker locker (&lock ());
+  if (m_invalid > 0) {
+    if (--m_invalid == 0) {
+      force_update_no_lock ();
+    }
+  }
+}
+
+void
+Layout::end_changes_no_update ()
+{
+  tl::MutexLocker locker (&lock ());
+  if (m_invalid > 0) {
+    --m_invalid;
+  }
+}
+
 void 
 Layout::force_update () 
+{
+  //  NOTE: the assumption is that either one thread is writing or
+  //  multiple threads are reading. Hence, we do not need to lock hier_dirty() or bboxes_dirty().
+  //  We still do double checking as another thread might do the update as well.
+  if (! hier_dirty () && ! bboxes_dirty ()) {
+    return;
+  }
+
+  tl::MutexLocker locker (&lock ());
+  force_update_no_lock ();
+}
+
+void
+Layout::force_update_no_lock () const
 {
   if (hier_dirty () || bboxes_dirty ()) {
 
@@ -1776,22 +1817,17 @@ Layout::force_update ()
 void 
 Layout::update () const
 {
-  if (! under_construction () && (hier_dirty () || bboxes_dirty ())) {
+  //  NOTE: the assumption is that either one thread is writing or
-
+  //  multiple threads are reading. Hence, we do not need to lock hier_dirty() or bboxes_dirty().
-    try {
+  //  We still do double checking as another thread might do the update as well.
-
+  if (under_construction () || (! hier_dirty () && ! bboxes_dirty ())) {
-      m_invalid = std::numeric_limits<unsigned int>::max ();   //  prevent recursion
+    return;
-
-      db::LayoutStateModel *state_model = const_cast<db::LayoutStateModel *> ((const db::LayoutStateModel *) this);
-      state_model->update ();
-
-      m_invalid = 0;
-
-    } catch (...) {
-      m_invalid = 0;
-      throw;
   }
 
+  tl::MutexLocker locker (&lock ());
+
+  if (! under_construction ()) {
+    force_update_no_lock ();
   }
 }
 

src/db/db/dbLayout.h

@@ -624,7 +624,7 @@ public:
    *  @brief Gets the lock for the layout object
    *  This is a generic lock that can be used to lock modifications against multiple threads.
    */
-  tl::Mutex &lock ()
+  tl::Mutex &lock () const
   {
     return m_lock;
   }
@@ -1791,34 +1791,18 @@ public:
    *  an operation, the start/end_changes method pair does not
    *  need to be called.
    */
-  void start_changes ()
+  void start_changes ();
-  {
-    ++m_invalid;
-  }
 
   /**
    *  @brief Cancel the "in changes" state (see "start_changes")
    */
-  void end_changes ()
+  void end_changes ();
-  {
-    if (m_invalid > 0) {
-      --m_invalid;
-      if (! m_invalid) {
-        update ();
-      }
-    }
-  }
 
   /**
    *  @brief Cancel the "in changes" state (see "start_changes")
    *  This version does not force an update
    */
-  void end_changes_no_update ()
+  void end_changes_no_update ();
-  {
-    if (m_invalid > 0) {
-      --m_invalid;
-    }
-  }
 
   /**
    *  @brief Tell if the layout object is under construction
@@ -2168,7 +2152,7 @@ private:
   std::map<db::cell_index_type, meta_info_map> m_meta_info_by_cell;
 
   std::string m_tech_name;
-  tl::Mutex m_lock;
+  mutable tl::Mutex m_lock;
 
   /**
    *  @brief Sort the cells topologically
@@ -2211,6 +2195,11 @@ private:
    *  @brief Recovers a proxy without considering the library from context_info
    */
   db::Cell *recover_proxy_no_lib (const LayoutOrCellContextInfo &context_info);
+
+  /**
+   *  @brief Does an update without checking under_construction() and no Mutex
+   */
+  void force_update_no_lock () const;
 };
 
 /**

src/db/db/dbLayoutStateModel.cc

@@ -29,13 +29,14 @@ namespace db
 {
 
 LayoutStateModel::LayoutStateModel (bool busy)
-  : m_hier_dirty (false), m_hier_generation_id (0), m_all_bboxes_dirty (false), m_busy (busy)
+  : m_hier_dirty (false), m_hier_generation_id (0), m_all_bboxes_dirty (false), m_some_bboxes_dirty (false), m_busy (busy)
 {
   //  .. nothing yet ..
 }
 
 LayoutStateModel::LayoutStateModel (const LayoutStateModel &d)
-  : m_hier_dirty (d.m_hier_dirty), m_hier_generation_id (d.m_hier_generation_id), m_bboxes_dirty (d.m_bboxes_dirty), m_all_bboxes_dirty (d.m_all_bboxes_dirty), m_busy (d.m_busy)
+  : m_hier_dirty (d.m_hier_dirty), m_hier_generation_id (d.m_hier_generation_id), m_bboxes_dirty (d.m_bboxes_dirty),
+    m_all_bboxes_dirty (d.m_all_bboxes_dirty), m_some_bboxes_dirty (d.m_some_bboxes_dirty), m_busy (d.m_busy)
 {
   //  .. nothing yet ..
 }
@@ -47,6 +48,7 @@ LayoutStateModel::operator= (const LayoutStateModel &d)
   m_hier_generation_id = d.m_hier_generation_id;
   m_bboxes_dirty = d.m_bboxes_dirty;
   m_all_bboxes_dirty = d.m_all_bboxes_dirty;
+  m_some_bboxes_dirty = d.m_some_bboxes_dirty;
   m_busy = d.m_busy;
   return *this;
 }
@@ -84,6 +86,7 @@ LayoutStateModel::invalidate_bboxes (unsigned int index)
         m_bboxes_dirty.resize (index + 1, false);
       }
       m_bboxes_dirty [index] = true;
+      m_some_bboxes_dirty = true;
     }
   }
 }
@@ -91,7 +94,7 @@ LayoutStateModel::invalidate_bboxes (unsigned int index)
 bool
 LayoutStateModel::bboxes_dirty () const
 {
-  return ! m_bboxes_dirty.empty () || m_all_bboxes_dirty;
+  return m_some_bboxes_dirty || m_all_bboxes_dirty;
 }
 
 void
@@ -100,6 +103,7 @@ LayoutStateModel::update ()
   if (bboxes_dirty () || m_hier_dirty) {
     do_update ();
     m_bboxes_dirty.clear ();
+    m_some_bboxes_dirty = false;
     m_all_bboxes_dirty = false;
     m_hier_dirty = false;
   }

src/db/db/dbLayoutStateModel.h

@@ -209,7 +209,7 @@ private:
   bool m_hier_dirty;
   size_t m_hier_generation_id;
   std::vector<bool> m_bboxes_dirty;
-  bool m_all_bboxes_dirty;
+  bool m_all_bboxes_dirty, m_some_bboxes_dirty;
   bool m_busy;
 
   void do_invalidate_hier ();

src/db/db/dbRecursiveShapeIterator.cc

@@ -703,8 +703,8 @@ RecursiveShapeIterator::next (RecursiveShapeReceiver *receiver)
     }
 
     if (at_end ()) {
-      //  release the layout lock if at end - this way, the shape iterator can be
+      //  Take this opportunity the release the layout lock.
-      //  held further, without blocking the layout.
+      //  This way, the shape iterator can be held further, without blocking the layout.
       m_locker = db::LayoutLocker ();
     }