[Lldb-commits] [lldb] 7f4cb2e - [lldb] Make the order in which ClusterManager calls destructors deterministic

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-02-03T13:33:44+01:00
New Revision: 7f4cb2e7b67afd177cf8f8939d4bf5dcff33f20b

URL: https://github.com/llvm/llvm-project/commit/7f4cb2e7b67afd177cf8f8939d4bf5dcff33f20b
DIFF: https://github.com/llvm/llvm-project/commit/7f4cb2e7b67afd177cf8f8939d4bf5dcff33f20b.diff

LOG: [lldb] Make the order in which ClusterManager calls destructors deterministic

Summary:
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.

Reviewers: labath, JDevlieghere

Reviewed By: labath

Subscribers: mgrang, abidh, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D73871

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/SharedCluster.h b/lldb/include/lldb/Utility/SharedCluster.h
index 71bbb334cff3..38b2d19c7093 100644
--- a/lldb/include/lldb/Utility/SharedCluster.h
+++ b/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 @@ template <class T> class ClusterManager {
   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 @@ template <class T> class ClusterManager {
 
   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 @@ template <class T> class ClusterManager {
 
   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;
 };