[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;
 };



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;
 };

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits