Fix for issue #28606. Removed loop which iterated over all threads in the thread registry to check …

runtime/vm/dart_api_state.h

Index: runtime/vm/dart_api_state.h
diff --git a/runtime/vm/dart_api_state.h b/runtime/vm/dart_api_state.h
index 7946ef8ea5bfcc50162865e52d0b0efcb7e5ee69..8844d162ebd73f56717ae58dedf9caca9693af55 100644
--- a/runtime/vm/dart_api_state.h
+++ b/runtime/vm/dart_api_state.h
@@ -456,6 +456,7 @@ class PersistentHandles : Handles<kPersistentHandleSizeInWords,
       : Handles<kPersistentHandleSizeInWords,
                 kPersistentHandlesPerChunk,
                 kOffsetOfRawPtrInPersistentHandle>(),
+        mutex_(new Mutex()),

[bkonyi, 2017/02/02 22:22:34]

It looks like PersistentHandles contains shared handles within the current
isolate, so I feel there might be the potential for a race condition here. I
noticed FinalizablePersistentHandles does have locks, so I've added them as
well. 

[bkonyi, 2017/02/02 22:43:03]

As discussed with Siva, this isn't needed. Removed in patch set 2.

[siva, 2017/02/02 22:51:13]

FinalizablePersistentHandles has a lock because we have background finalization
done in a different thread.

PersistentHandles should only be accessed from the mutator thread.

         free_list_(NULL) {
     if (FLAG_trace_handles) {
       OS::PrintErr("*** Starting a new Persistent handle block 0x%" Px "\n",
@@ -463,6 +464,7 @@ class PersistentHandles : Handles<kPersistentHandleSizeInWords,
     }
   }
   ~PersistentHandles() {
+    delete mutex_;
     free_list_ = NULL;
     if (FLAG_trace_handles) {
       OS::PrintErr("***   Handle Counts for 0x(%" Px "):Scoped = %d\n",
@@ -478,12 +480,14 @@ class PersistentHandles : Handles<kPersistentHandleSizeInWords,
 
   // Visit all object pointers stored in the various handles.
   void VisitObjectPointers(ObjectPointerVisitor* visitor) {
+    MutexLocker ml(mutex_);
     Handles<kPersistentHandleSizeInWords, kPersistentHandlesPerChunk,
             kOffsetOfRawPtrInPersistentHandle>::VisitObjectPointers(visitor);
   }
 
   // Visit all the handles.
   void Visit(HandleVisitor* visitor) {
+    MutexLocker ml(mutex_);
     Handles<kPersistentHandleSizeInWords, kPersistentHandlesPerChunk,
             kOffsetOfRawPtrInPersistentHandle>::Visit(visitor);
   }
@@ -491,6 +495,7 @@ class PersistentHandles : Handles<kPersistentHandleSizeInWords,
   // Allocates a persistent handle, these have to be destroyed explicitly
   // by calling FreeHandle.
   PersistentHandle* AllocateHandle() {
+    MutexLocker ml(mutex_);
     PersistentHandle* handle;
     if (free_list_ != NULL) {
       handle = free_list_;
@@ -503,16 +508,19 @@ class PersistentHandles : Handles<kPersistentHandleSizeInWords,
   }
 
   void FreeHandle(PersistentHandle* handle) {
+    MutexLocker ml(mutex_);
     handle->FreeHandle(free_list());
     set_free_list(handle);
   }
 
   // Validate if passed in handle is a Persistent Handle.
   bool IsValidHandle(Dart_PersistentHandle object) const {
+    MutexLocker ml(mutex_);
     return IsValidScopedHandle(reinterpret_cast<uword>(object));
   }
 
   bool IsFreeHandle(Dart_PersistentHandle object) const {
+    MutexLocker ml(mutex_);
     PersistentHandle* handle = free_list_;
     while (handle != NULL) {
       if (handle == reinterpret_cast<PersistentHandle*>(object)) {
@@ -527,6 +535,7 @@ class PersistentHandles : Handles<kPersistentHandleSizeInWords,
   int CountHandles() const { return CountScopedHandles(); }
 
  private:
+  Mutex* mutex_;
   PersistentHandle* free_list_;
   DISALLOW_COPY_AND_ASSIGN(PersistentHandles);
 };