commit cc3bf609f26bb33d1ecbd8c86b4c6932734ece73
author Ryan Macnak <rmacnak@google.com> Tue Apr 25 22:10:09 2023 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Apr 25 22:10:09 2023 +0000
tree fe648c8985b3f5cedf83eca6e145af67e1c4ac3f
parent 5138a1eb383871922e20acafa3b8a9e0b17ed073

[vm, gc] Avoid suprious race between GC helper thread exit and GC visiting roots.

A sweeper helper thread may be exiting while the scavenge is visiting thread roots. GC helpers have no reusable handle state to clear, so the race is an uninteresting one between null and null.

TEST=tsan
Bug: https://github.com/dart-lang/sdk/issues/52125
Change-Id: Iae68f8a3a938ae20e540b11c172fbc4a50a90a9c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298262
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>

runtime/vm/thread.cc

diff --git a/runtime/vm/thread.cc b/runtime/vm/thread.cc
index dc7d87c..7ee53d36 100644
--- a/runtime/vm/thread.cc
+++ b/runtime/vm/thread.cc
@@ -348,6 +348,9 @@
     ASSERT(global_object_pool_ == Object::null());
     ASSERT(ffi_callback_code_ == Object::null());
     ASSERT(ffi_callback_stack_return_ == Object::null());
+#define CHECK_REUSABLE_HANDLE(object) ASSERT(object##_handle_->IsNull());
+    REUSABLE_HANDLE_LIST(CHECK_REUSABLE_HANDLE)
+#undef CHECK_REUSABLE_HANDLE
   }
 }
 
@@ -598,7 +601,13 @@
   ASSERT(!thread->HasActiveState());
   ASSERT(!thread->IsAtSafepoint());
 
-  thread->ClearReusableHandles();
+  if (!bypass_safepoint) {
+    // GC helper threads don't have any handle state to clear, and the GC might
+    // be currently visiting thread state. If this is not a GC helper, the GC
+    // can't be visiting thread state because its waiting for this thread to
+    // check in.
+    thread->ClearReusableHandles();
+  }
 
   auto group = thread->isolate_group_;
   auto thread_registry = group->thread_registry();