[vm, gc] Don't start or finalize concurrent marking during a force gr…

…owth scope.

Adjust tests to exit force growth scopes before forcing a GC.

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

runtime/vm/dart_api_impl_test.cc

@@ -3133,10 +3133,6 @@ static void WeakPersistentHandleCallback(void* isolate_callback_data,
 }
 
 TEST_CASE(DartAPI_WeakPersistentHandle) {
-  // GCs due to allocations or weak handle creation can cause early promotion
-  // and interfer with the scenario this test is verifying.
-  NoHeapGrowthControlScope force_growth;
-
   Dart_Handle local_new_ref = Dart_Null();
   weak_new_ref = Dart_NewWeakPersistentHandle(local_new_ref, NULL, 0,
                                               WeakPersistentHandleCallback);
@@ -3148,25 +3144,32 @@ TEST_CASE(DartAPI_WeakPersistentHandle) {
   {
     Dart_EnterScope();
 
-    // Create an object in new space.
-    Dart_Handle new_ref = AllocateNewString("new string");
-    EXPECT_VALID(new_ref);
-
-    // Create an object in old space.
-    Dart_Handle old_ref = AllocateOldString("old string");
-    EXPECT_VALID(old_ref);
-
-    // Create a weak ref to the new space object.
-    weak_new_ref = Dart_NewWeakPersistentHandle(new_ref, NULL, 0,
-                                                WeakPersistentHandleCallback);
-    EXPECT_VALID(AsHandle(weak_new_ref));
-    EXPECT(!Dart_IsNull(AsHandle(weak_new_ref)));
-
-    // Create a weak ref to the old space object.
-    weak_old_ref = Dart_NewWeakPersistentHandle(old_ref, NULL, 0,
-                                                WeakPersistentHandleCallback);
-    EXPECT_VALID(AsHandle(weak_old_ref));
-    EXPECT(!Dart_IsNull(AsHandle(weak_old_ref)));
+    Dart_Handle new_ref, old_ref;
+    {
+      // GCs due to allocations or weak handle creation can cause early
+      // promotion and interfere with the scenario this test is verifying.
+      NoHeapGrowthControlScope force_growth;
+
+      // Create an object in new space.
+      new_ref = AllocateNewString("new string");
+      EXPECT_VALID(new_ref);
+
+      // Create an object in old space.
+      old_ref = AllocateOldString("old string");
+      EXPECT_VALID(old_ref);
+
+      // Create a weak ref to the new space object.
+      weak_new_ref = Dart_NewWeakPersistentHandle(new_ref, NULL, 0,
+                                                  WeakPersistentHandleCallback);
+      EXPECT_VALID(AsHandle(weak_new_ref));
+      EXPECT(!Dart_IsNull(AsHandle(weak_new_ref)));
+
+      // Create a weak ref to the old space object.
+      weak_old_ref = Dart_NewWeakPersistentHandle(old_ref, NULL, 0,
+                                                  WeakPersistentHandleCallback);
+      EXPECT_VALID(AsHandle(weak_old_ref));
+      EXPECT(!Dart_IsNull(AsHandle(weak_old_ref)));
+    }
 
     {
       TransitionNativeToVM transition(thread);
@@ -3264,10 +3267,6 @@ static void FinalizableHandleCallback(void* isolate_callback_data, void* peer) {
 }
 
 TEST_CASE(DartAPI_FinalizableHandle) {
-  // GCs due to allocations or weak handle creation can cause early promotion
-  // and interfer with the scenario this test is verifying.
-  NoHeapGrowthControlScope force_growth;
-
   void* peer = reinterpret_cast<void*>(0);
   Dart_Handle local_new_ref = Dart_Null();
   finalizable_new_ref = Dart_NewFinalizableHandle(local_new_ref, peer, 0,
@@ -3283,25 +3282,32 @@ TEST_CASE(DartAPI_FinalizableHandle) {
   {
     Dart_EnterScope();
 
-    // Create an object in new space.
-    Dart_Handle new_ref = AllocateNewString("new string");
-    EXPECT_VALID(new_ref);
-
-    // Create an object in old space.
-    Dart_Handle old_ref = AllocateOldString("old string");
-    EXPECT_VALID(old_ref);
-
-    // Create a weak ref to the new space object.
-    peer = reinterpret_cast<void*>(2);
-    finalizable_new_ref =
-        Dart_NewFinalizableHandle(new_ref, peer, 0, FinalizableHandleCallback);
-    finalizable_new_ref_peer = peer;
-
-    // Create a weak ref to the old space object.
-    peer = reinterpret_cast<void*>(3);
-    finalizable_old_ref =
-        Dart_NewFinalizableHandle(old_ref, peer, 0, FinalizableHandleCallback);
-    finalizable_old_ref_peer = peer;
+    Dart_Handle new_ref, old_ref;
+    {
+      // GCs due to allocations or weak handle creation can cause early
+      // promotion and interfere with the scenario this test is verifying.
+      NoHeapGrowthControlScope force_growth;
+
+      // Create an object in new space.
+      new_ref = AllocateNewString("new string");
+      EXPECT_VALID(new_ref);
+
+      // Create an object in old space.
+      old_ref = AllocateOldString("old string");
+      EXPECT_VALID(old_ref);
+
+      // Create a weak ref to the new space object.
+      peer = reinterpret_cast<void*>(2);
+      finalizable_new_ref = Dart_NewFinalizableHandle(
+          new_ref, peer, 0, FinalizableHandleCallback);
+      finalizable_new_ref_peer = peer;
+
+      // Create a weak ref to the old space object.
+      peer = reinterpret_cast<void*>(3);
+      finalizable_old_ref = Dart_NewFinalizableHandle(
+          old_ref, peer, 0, FinalizableHandleCallback);
+      finalizable_old_ref_peer = peer;
+    }
 
     {
       TransitionNativeToVM transition(thread);

runtime/vm/heap/heap.cc

@@ -111,15 +111,13 @@ uword Heap::AllocateNew(intptr_t size) {
 
 uword Heap::AllocateOld(intptr_t size, OldPage::PageType type) {
   ASSERT(Thread::Current()->no_safepoint_scope_depth() == 0);
-  CollectForDebugging();
-  uword addr = old_space_.TryAllocate(size, type);
-  if (addr != 0) {
-    return addr;
-  }
-  // If we are in the process of running a sweep, wait for the sweeper to free
-  // memory.
-  Thread* thread = Thread::Current();
   if (old_space_.GrowthControlState()) {
+    CollectForDebugging();
+    uword addr = old_space_.TryAllocate(size, type);
+    if (addr != 0) {
+      return addr;
+    }
+    Thread* thread = Thread::Current();
     // Wait for any GC tasks that are in progress.
     WaitForSweeperTasks(thread);
     addr = old_space_.TryAllocate(size, type);
@@ -148,7 +146,7 @@ uword Heap::AllocateOld(intptr_t size, OldPage::PageType type) {
     CollectAllGarbage(kLowMemory);
     WaitForSweeperTasks(thread);
   }
-  addr = old_space_.TryAllocate(size, type, PageSpace::kForceGrowth);
+  uword addr = old_space_.TryAllocate(size, type, PageSpace::kForceGrowth);
   if (addr != 0) {
     return addr;
   }

runtime/vm/heap/pages.cc

@@ -452,6 +452,7 @@ void PageSpace::FreePages(OldPage* pages) {
 
 void PageSpace::EvaluateConcurrentMarking(GrowthPolicy growth_policy) {
   if (growth_policy != kForceGrowth) {
+    ASSERT(GrowthControlState());
     if (heap_ != NULL) {  // Some unit tests.
       Thread* thread = Thread::Current();
       if (thread->CanCollectGarbage()) {
@@ -1020,6 +1021,8 @@ bool PageSpace::ShouldPerformIdleMarkCompact(int64_t deadline) {
 }
 
 void PageSpace::CollectGarbage(bool compact, bool finalize) {
+  ASSERT(GrowthControlState());
+
   if (!finalize) {
 #if defined(TARGET_ARCH_IA32)
     return;  // Barrier not implemented.

runtime/vm/heap/pages_test.cc

@@ -10,6 +10,7 @@ namespace dart {
 
 TEST_CASE(Pages) {
   PageSpace* space = new PageSpace(NULL, 4 * MBInWords);
+  space->InitGrowthControl();
   EXPECT(!space->Contains(reinterpret_cast<uword>(&space)));
   uword block = space->TryAllocate(8 * kWordSize);
   EXPECT(block != 0);

runtime/vm/thread_test.cc

@@ -176,8 +176,6 @@ ISOLATE_UNIT_TEST_CASE(ManyTasksWithZones) {
   Monitor sync[kTaskCount];
   bool done[kTaskCount];
   Isolate* isolate = thread->isolate();
-  EXPECT(isolate->heap()->GrowthControlState());
-  isolate->heap()->DisableGrowthControl();
   for (int i = 0; i < kTaskCount; i++) {
     done[i] = false;
     Dart::thread_pool()->Run<TaskWithZoneAllocation>(isolate, &sync[i],