[Lldb-commits] [PATCH] D73871: [lldb] Make the order in which ClusterManager calls destructors deterministic

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: labath, JDevlieghere.
Herald added subscribers: lldb-commits, abidh, mgrang.
Herald added a project: LLDB.

ClusterManager is using a SmallPtrSet to store the objects in it. We always only add every object once so using a set is not necessary.
Furthermore having a set means that iterating over it is nondeterministic (at least with more than 16 objects in it), so the order in
which the destructors for the managed objects are called is currently also non-deterministic.

This just replaces the SmallPtrSet with a SmallVector.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D73871

Files:
  lldb/include/lldb/Utility/SharedCluster.h


Index: lldb/include/lldb/Utility/SharedCluster.h
===================================================================
--- lldb/include/lldb/Utility/SharedCluster.h
+++ lldb/include/lldb/Utility/SharedCluster.h
@@ -12,7 +12,8 @@
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/SharingPtr.h"
 
-#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
 
 #include <mutex>
 
@@ -42,12 +43,8 @@
   ClusterManager() : m_objects(), m_external_ref(0), m_mutex() {}
 
   ~ClusterManager() {
-    for (typename llvm::SmallPtrSet<T *, 16>::iterator pos = m_objects.begin(),
-                                                       end = m_objects.end();
-         pos != end; ++pos) {
-      T *object = *pos;
-      delete object;
-    }
+    for (T *obj : m_objects)
+      delete obj;
 
     // Decrement refcount should have been called on this ClusterManager, and
     // it should have locked the mutex, now we will unlock it before we destroy
@@ -57,14 +54,16 @@
 
   void ManageObject(T *new_object) {
     std::lock_guard<std::mutex> guard(m_mutex);
-    m_objects.insert(new_object);
+    assert(!llvm::is_contained(m_objects, new_object) &&
+           "ManageObject called twice for the same object?");
+    m_objects.push_back(new_object);
   }
 
   typename lldb_private::SharingPtr<T> GetSharedPointer(T *desired_object) {
     {
       std::lock_guard<std::mutex> guard(m_mutex);
       m_external_ref++;
-      if (0 == m_objects.count(desired_object)) {
+      if (!llvm::is_contained(m_objects, desired_object)) {
         lldbassert(false && "object not found in shared cluster when expected");
         desired_object = nullptr;
       }
@@ -85,7 +84,7 @@
 
   friend class imp::shared_ptr_refcount<ClusterManager>;
 
-  llvm::SmallPtrSet<T *, 16> m_objects;
+  llvm::SmallVector<T *, 16> m_objects;
   int m_external_ref;
   std::mutex m_mutex;
 };


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73871.242013.patch
Type: text/x-patch
Size: 1918 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200203/4aa7a402/attachment-0001.bin>