[vm] Check Zones are only [un]chained on the correct side of a safepoint transition.
Fix order in Dart_CreateIsolate and some tests.
Change-Id: Iceb748be3f7aa6b68ce7abb7f65a3c54ad63c1d8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/100795
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc
index ec07f5a..8ef0473 100644
--- a/runtime/vm/dart_api_impl.cc
+++ b/runtime/vm/dart_api_impl.cc
@@ -1093,8 +1093,10 @@
}
return reinterpret_cast<Dart_Isolate>(NULL);
}
+
+ Thread* T = Thread::Current();
+ bool success = false;
{
- Thread* T = Thread::Current();
StackZone zone(T);
HANDLESCOPE(T);
// We enter an API scope here as InitializeIsolate could compile some
@@ -1112,25 +1114,27 @@
Library::CheckFunctionFingerprints();
}
#endif // defined(DART_NO_SNAPSHOT) && !defined(PRODUCT).
- // We exit the API scope entered above.
- T->ExitApiScope();
- // A Thread structure has been associated to the thread, we do the
- // safepoint transition explicitly here instead of using the
- // TransitionXXX scope objects as the reverse transition happens
- // outside this scope in Dart_ShutdownIsolate/Dart_ExitIsolate.
- T->set_execution_state(Thread::kThreadInNative);
- T->EnterSafepoint();
- if (error != NULL) {
- *error = NULL;
- }
- return Api::CastIsolate(I);
- }
- if (error != NULL) {
+ success = true;
+ } else if (error != NULL) {
*error = strdup(error_obj.ToErrorCString());
}
// We exit the API scope entered above.
T->ExitApiScope();
}
+
+ if (success) {
+ // A Thread structure has been associated to the thread, we do the
+ // safepoint transition explicitly here instead of using the
+ // TransitionXXX scope objects as the reverse transition happens
+ // outside this scope in Dart_ShutdownIsolate/Dart_ExitIsolate.
+ T->set_execution_state(Thread::kThreadInNative);
+ T->EnterSafepoint();
+ if (error != NULL) {
+ *error = NULL;
+ }
+ return Api::CastIsolate(I);
+ }
+
Dart::ShutdownIsolate();
return reinterpret_cast<Dart_Isolate>(NULL);
}
diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc
index 406901a..0682bb9 100644
--- a/runtime/vm/dart_api_impl_test.cc
+++ b/runtime/vm/dart_api_impl_test.cc
@@ -3469,8 +3469,8 @@
ApiLocalScope* scope = thread->api_top_scope();
Dart_Handle handles[300];
{
- StackZone zone(thread);
TransitionNativeToVM transition1(thread);
+ StackZone zone(thread);
HANDLESCOPE(thread);
Smi& val = Smi::Handle();
TransitionVMToNative transition2(thread);
diff --git a/runtime/vm/snapshot_test.cc b/runtime/vm/snapshot_test.cc
index ec774c5..d11d4213 100644
--- a/runtime/vm/snapshot_test.cc
+++ b/runtime/vm/snapshot_test.cc
@@ -2030,7 +2030,7 @@
Dart_ExitScope();
}
-TEST_CASE(OmittedObjectEncodingLength) {
+ISOLATE_UNIT_TEST_CASE(OmittedObjectEncodingLength) {
StackZone zone(Thread::Current());
MessageWriter writer(true);
writer.WriteInlinedObjectHeader(kOmittedObjectId);
diff --git a/runtime/vm/thread_test.cc b/runtime/vm/thread_test.cc
index 9a42800..1030795 100644
--- a/runtime/vm/thread_test.cc
+++ b/runtime/vm/thread_test.cc
@@ -376,6 +376,7 @@
TestCase::CreateTestIsolate();
{
// Create a stack resource this time, and exercise it.
+ TransitionNativeToVM transition(Thread::Current());
StackZone stack_zone(Thread::Current());
Zone* zone1 = Thread::Current()->zone();
EXPECT(zone1 != zone0);
diff --git a/runtime/vm/zone.cc b/runtime/vm/zone.cc
index 2d95479..595871a 100644
--- a/runtime/vm/zone.cc
+++ b/runtime/vm/zone.cc
@@ -337,11 +337,20 @@
reinterpret_cast<intptr_t>(this),
reinterpret_cast<intptr_t>(&zone_));
}
+
+ // This thread must be preventing safepoints or the GC could be visiting the
+ // chain of handle blocks we're about the mutate.
+ ASSERT(Thread::Current()->MayAllocateHandles());
+
zone_.Link(thread->zone());
thread->set_zone(&zone_);
}
StackZone::~StackZone() {
+ // This thread must be preventing safepoints or the GC could be visiting the
+ // chain of handle blocks we're about the mutate.
+ ASSERT(Thread::Current()->MayAllocateHandles());
+
ASSERT(thread()->zone() == &zone_);
thread()->set_zone(zone_.previous_);
if (FLAG_trace_zones) {
diff --git a/runtime/vm/zone_test.cc b/runtime/vm/zone_test.cc
index aeb98d6..e52b940 100644
--- a/runtime/vm/zone_test.cc
+++ b/runtime/vm/zone_test.cc
@@ -18,6 +18,7 @@
Thread* thread = Thread::Current();
EXPECT(thread->zone() == NULL);
{
+ TransitionNativeToVM transition(thread);
StackZone stack_zone(thread);
EXPECT(thread->zone() != NULL);
Zone* zone = stack_zone.GetZone();
@@ -78,6 +79,7 @@
Thread* thread = Thread::Current();
EXPECT(thread->zone() == NULL);
{
+ TransitionNativeToVM transition(thread);
StackZone zone(thread);
EXPECT(thread->zone() != NULL);
uintptr_t allocated_size = 0;
@@ -131,6 +133,7 @@
// Create a few zone allocated objects.
{
+ TransitionNativeToVM transition(thread);
StackZone zone(thread);
EXPECT_EQ(0UL, zone.SizeInBytes());
SimpleZoneObject* first = new SimpleZoneObject();
@@ -156,6 +159,7 @@
}
TEST_CASE(PrintToString) {
+ TransitionNativeToVM transition(Thread::Current());
StackZone zone(Thread::Current());
const char* result = zone.GetZone()->PrintToString("Hello %s!", "World");
EXPECT_STREQ("Hello World!", result);
@@ -219,7 +223,7 @@
#endif // !defined(PRODUCT)
}
-TEST_CASE(StressMallocThroughZones) {
+ISOLATE_UNIT_TEST_CASE(StressMallocThroughZones) {
#if !defined(PRODUCT)
int64_t start_rss = Service::CurrentRSS();
#endif // !defined(PRODUCT)
