commit af50a684d12a69fc90a06e394a37adfb957abaa9
author Alexander Aprelev <aam@google.com> Mon Aug 02 14:59:10 2021 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Mon Aug 02 14:59:10 2021 +0000
tree 93aaaaa932e3be0c5d1d3567d94dbd2ba73174b0
parent 4a714b27d60c461c17c9b6f4b63ebc93dc9d543d

[vm/concurrency] Grab constant_canonicalization_mutex before entering Safepoint scope.

If we attempt to acquire mutex once we are in Safepoint scope, we might never get it because some other thread force to a safepoint is actually holding that mutex already.

Fixes https://github.com/dart-lang/sdk/issues/46732

TEST=issue_6610_test on debug build

Change-Id: I4fc7154aae075c608af8facf53a029ddf5d0de2c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208520
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>

runtime/vm/isolate.cc

diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc
index d7e8fcb..b1a7315 100644
--- a/runtime/vm/isolate.cc
+++ b/runtime/vm/isolate.cc
@@ -988,6 +988,8 @@
   NoBackgroundCompilerScope no_bg_compiler(Thread::Current());
   heap()->CollectAllGarbage();
   Thread* thread = Thread::Current();
+  SafepointMutexLocker ml(
+      thread->isolate_group()->constant_canonicalization_mutex());
   HeapIterationScope iteration(thread);
   VerifyCanonicalVisitor check_canonical(thread);
   iteration.IterateObjects(&check_canonical);

runtime/vm/object.cc

diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 0df5c45..a1b8649 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -19122,8 +19122,9 @@
   Zone* zone = thread->zone();
   Instance& result = Instance::Handle(zone);
   const Class& cls = Class::Handle(zone, this->clazz());
-  SafepointMutexLocker ml(
-      thread->isolate_group()->constant_canonicalization_mutex());
+  ASSERT(thread->isolate_group()
+             ->constant_canonicalization_mutex()
+             ->IsOwnedByCurrentThread());
   result ^= cls.LookupCanonicalInstance(zone, *this);
   return (result.ptr() == this->ptr());
 }
@@ -21100,7 +21101,9 @@
 
   ObjectStore* object_store = isolate_group->object_store();
   {
-    SafepointMutexLocker ml(isolate_group->type_canonicalization_mutex());
+    ASSERT(thread->isolate_group()
+               ->constant_canonicalization_mutex()
+               ->IsOwnedByCurrentThread());
     CanonicalTypeSet table(zone, object_store->canonical_types());
     type ^= table.GetOrNull(CanonicalTypeKey(*this));
     object_store->set_canonical_types(table.Release());
@@ -21457,7 +21460,9 @@
   FunctionType& type = FunctionType::Handle(zone);
   ObjectStore* object_store = isolate_group->object_store();
   {
-    SafepointMutexLocker ml(isolate_group->type_canonicalization_mutex());
+    ASSERT(thread->isolate_group()
+               ->constant_canonicalization_mutex()
+               ->IsOwnedByCurrentThread());
     CanonicalFunctionTypeSet table(zone,
                                    object_store->canonical_function_types());
     type ^= table.GetOrNull(CanonicalFunctionTypeKey(*this));
@@ -22008,7 +22013,9 @@
   TypeParameter& type_parameter = TypeParameter::Handle(zone);
   ObjectStore* object_store = isolate_group->object_store();
   {
-    SafepointMutexLocker ml(isolate_group->type_canonicalization_mutex());
+    ASSERT(thread->isolate_group()
+               ->constant_canonicalization_mutex()
+               ->IsOwnedByCurrentThread());
     CanonicalTypeParameterSet table(zone,
                                     object_store->canonical_type_parameters());
     type_parameter ^= table.GetOrNull(CanonicalTypeParameterKey(*this));