Treat background finalization as another GC task so it won't run in parallel with the marker.

runtime/vm/dart_api_impl_test.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "bin/builtin.h"
 #include "vm/compiler.h"
 #include "include/dart_api.h"
 #include "include/dart_mirrors_api.h"
 #include "include/dart_native_api.h"
 #include "include/dart_tools_api.h"
@@ skipping 1199 matching lines @@
 // The normal call to CollectGarbage(Heap::kNew) could potentially trigger
 // an old gen collection if there is a promotion failure and this could
 // perturb the test.
 class GCTestHelper : public AllStatic {
  public:
   static void CollectNewSpace(Heap::ApiCallbacks api_callbacks) {
     bool invoke_api_callbacks = (api_callbacks == Heap::kInvokeApiCallbacks);
     Isolate::Current()->heap()->new_space()->Scavenge(invoke_api_callbacks);
   }
 
-  static void WaitForFinalizationTasks() {
+  static void WaitForGCTasks() {
     Thread* thread = Thread::Current();
-    Heap* heap = thread->isolate()->heap();
-    MonitorLocker ml(heap->finalization_tasks_lock());
-    while (heap->finalization_tasks() > 0) {
+    PageSpace* old_space = thread->isolate()->heap()->old_space();
+    MonitorLocker ml(old_space->tasks_lock());
+    while (old_space->tasks() > 0) {
       ml.WaitWithSafepointCheck(thread);
     }
   }
 };
 
 
 static void ExternalStringCallbackFinalizer(void* peer) {
   *static_cast<int*>(peer) *= 2;
 }
 
@@ skipping 22 matching lines @@
     EXPECT_VALID(obj16);
 
     Dart_ExitScope();
   }
 
   {
     TransitionNativeToVM transition(thread);
     EXPECT_EQ(40, peer8);
     EXPECT_EQ(41, peer16);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT_EQ(40, peer8);
     EXPECT_EQ(41, peer16);
     Isolate::Current()->heap()->CollectGarbage(Heap::kNew);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT_EQ(80, peer8);
     EXPECT_EQ(82, peer16);
   }
 }
 
 
 TEST_CASE(ExternalStringPretenure) {
   {
     Dart_EnterScope();
     static const uint8_t big_data8[16*MB] = {0, };
@@ skipping 1091 matching lines @@
   EXPECT(value);
 }
 
 
 static void NopCallback(void* isolate_callback_data,
                         Dart_WeakPersistentHandle handle,
                         void* peer) {
 }
 
 
+static void UnreachedCallback(void* isolate_callback_data,
+                              Dart_WeakPersistentHandle handle,
+                              void* peer) {
+  UNREACHABLE();
+}
+
+
 static void ExternalTypedDataFinalizer(void* isolate_callback_data,
                                        Dart_WeakPersistentHandle handle,
                                        void* peer) {
   *static_cast<int*>(peer) = 42;
 }
 
 
 TEST_CASE(ExternalTypedDataCallback) {
   int peer = 0;
   {
     Dart_EnterScope();
     uint8_t data[] = { 1, 2, 3, 4 };
     Dart_Handle obj = Dart_NewExternalTypedData(
         Dart_TypedData_kUint8,
         data,
         ARRAY_SIZE(data));
     Dart_NewWeakPersistentHandle(
         obj, &peer, sizeof(data), ExternalTypedDataFinalizer);
     EXPECT_VALID(obj);
     Dart_ExitScope();
   }
   {
     TransitionNativeToVM transition(thread);
     EXPECT(peer == 0);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT(peer == 0);
     Isolate::Current()->heap()->CollectGarbage(Heap::kNew);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT(peer == 42);
   }
 }
 
 
-static Monitor* slow_finalizers_monitor = NULL;
-static intptr_t slow_finalizers_waiting = 0;
-
-
 static void SlowFinalizer(void* isolate_callback_data,
                           Dart_WeakPersistentHandle handle,
                           void* peer) {
-  {
-    MonitorLocker ml(slow_finalizers_monitor);
-    slow_finalizers_waiting++;
-    while (slow_finalizers_waiting < 10) {
-      ml.Wait();
-    }
-    ml.NotifyAll();
-  }
-
+  OS::Sleep(10);

[siva, 2016/08/29 22:05:09]

Why is this sleep necessary as you now make sure only one set of finalizer is
running?

[rmacnak, 2016/08/29 23:09:45]

Trying to stress the situation where the marker has to wait.

   intptr_t* count = reinterpret_cast<intptr_t*>(peer);
-  AtomicOperations::IncrementBy(count, 1);
+  (*count)++;
 }
 
 
 TEST_CASE(SlowFinalizer) {
-  slow_finalizers_monitor = new Monitor();
-
   intptr_t count = 0;
   for (intptr_t i = 0; i < 10; i++) {
     Dart_EnterScope();
     Dart_Handle str1 = Dart_NewStringFromCString("Live fast");
     Dart_NewWeakPersistentHandle(str1, &count, 0, SlowFinalizer);
     Dart_Handle str2 = Dart_NewStringFromCString("Die young");
     Dart_NewWeakPersistentHandle(str2, &count, 0, SlowFinalizer);
     Dart_ExitScope();
 
     {
       TransitionNativeToVM transition(thread);
       Isolate::Current()->heap()->CollectAllGarbage();
     }
   }
 
   {
     TransitionNativeToVM transition(thread);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
   }
 
   EXPECT_EQ(20, count);
-
-  delete slow_finalizers_monitor;
 }
 
 
 static void CheckFloat32x4Data(Dart_Handle obj) {
   void* raw_data = NULL;
   intptr_t len;
   Dart_TypedData_Type type;
   EXPECT_VALID(Dart_TypedDataAcquireData(obj, &type, &raw_data, &len));
   EXPECT_EQ(Dart_TypedData_kFloat32x4, type);
   EXPECT_EQ(len, 10);
@@ skipping 34 matching lines @@
     Dart_Handle lcl = Dart_NewExternalTypedData(
         Dart_TypedData_kFloat32x4, data, 10);
     Dart_NewWeakPersistentHandle(
         lcl, &peer, sizeof(data), ExternalTypedDataFinalizer);
     CheckFloat32x4Data(lcl);
   }
   Dart_ExitScope();
   {
     TransitionNativeToVM transition(thread);
     Isolate::Current()->heap()->CollectGarbage(Heap::kNew);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT(peer == 42);
   }
 }
 
 
 // Unit test for entering a scope, creating a local handle and exiting
 // the scope.
 UNIT_TEST_CASE(EnterExitScope) {
   TestIsolateScope __test_isolate__;
 
@@ skipping 256 matching lines @@
     EXPECT(Dart_IdentityEquals(old_ref, AsHandle(weak_old_ref)));
 
     // Delete local (strong) references.
     Dart_ExitScope();
   }
 
   {
     TransitionNativeToVM transition(thread);
     // Garbage collect new space again.
     GCTestHelper::CollectNewSpace(Heap::kIgnoreApiCallbacks);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
   }
 
   {
     Dart_EnterScope();
     // Weak ref to new space object should now be cleared.
     EXPECT(weak_new_ref == NULL);
     EXPECT_VALID(AsHandle(weak_old_ref));
     EXPECT(!Dart_IsNull(AsHandle(weak_old_ref)));
     Dart_ExitScope();
   }
 
   {
     TransitionNativeToVM transition(thread);
     // Garbage collect old space again.
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
   }
 
   {
     Dart_EnterScope();
     // Weak ref to old space object should now be cleared.
     EXPECT(weak_new_ref == NULL);
     EXPECT(weak_old_ref == NULL);
     Dart_ExitScope();
   }
 
   {
     TransitionNativeToVM transition(thread);
     // Garbage collect one last time to revisit deleted handles.
     Isolate::Current()->heap()->CollectGarbage(Heap::kNew);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
   }
 }
 
 
+TEST_CASE(WeakPersistentHandleErrors) {
+  Dart_EnterScope();
+
+  // NULL callback.
+  Dart_Handle obj1 = NewString("new string");
+  EXPECT_VALID(obj1);
+  Dart_WeakPersistentHandle ref1 = Dart_NewWeakPersistentHandle(
+      obj1, NULL, 0, NULL);
+  EXPECT_EQ(ref1, static_cast<void*>(NULL));
+
+  // Immediate object.
+  Dart_Handle obj2 = Dart_NewInteger(0);
+  EXPECT_VALID(obj2);
+  Dart_WeakPersistentHandle ref2 = Dart_NewWeakPersistentHandle(
+      obj2, NULL, 0, WeakPersistentHandleCallback);
+  EXPECT_EQ(ref2, static_cast<void*>(NULL));
+
+  Dart_ExitScope();
+}
+
+
 static void WeakPersistentHandlePeerFinalizer(void* isolate_callback_data,
                                               Dart_WeakPersistentHandle handle,
                                               void* peer) {
   *static_cast<int*>(peer) = 42;
 }
 
 
 TEST_CASE(WeakPersistentHandleCallback) {
   Dart_WeakPersistentHandle weak_ref = NULL;
   int peer = 0;
   {
     Dart_EnterScope();
     Dart_Handle obj = NewString("new string");
     EXPECT_VALID(obj);
     weak_ref = Dart_NewWeakPersistentHandle(obj, &peer, 0,
                                             WeakPersistentHandlePeerFinalizer);
     EXPECT_VALID(AsHandle(weak_ref));
     EXPECT(peer == 0);
     Dart_ExitScope();
   }
   {
     TransitionNativeToVM transition(thread);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
     EXPECT(peer == 0);
     GCTestHelper::CollectNewSpace(Heap::kIgnoreApiCallbacks);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT(peer == 42);
   }
 }
 
 
 TEST_CASE(WeakPersistentHandleNoCallback) {
   Dart_WeakPersistentHandle weak_ref = NULL;
   int peer = 0;
   {
     Dart_EnterScope();
     Dart_Handle obj = NewString("new string");
     EXPECT_VALID(obj);
     weak_ref = Dart_NewWeakPersistentHandle(obj, &peer, 0,
                                             WeakPersistentHandlePeerFinalizer);
     Dart_ExitScope();
   }
   // A finalizer is not invoked on a deleted handle.  Therefore, the
   // peer value should not change after the referent is collected.
   Dart_Isolate isolate = reinterpret_cast<Dart_Isolate>(Isolate::Current());
   Dart_DeleteWeakPersistentHandle(isolate, weak_ref);
   EXPECT(peer == 0);
   {
     TransitionNativeToVM transition(thread);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
     EXPECT(peer == 0);
     GCTestHelper::CollectNewSpace(Heap::kIgnoreApiCallbacks);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT(peer == 0);
   }
 }
 
 
 UNIT_TEST_CASE(WeakPersistentHandlesCallbackShutdown) {
   TestCase::CreateTestIsolate();
   Dart_EnterScope();
   Dart_Handle ref = Dart_True();
   int peer = 1234;
@@ skipping 40 matching lines @@
     Dart_ExitScope();
   }
   {
     TransitionNativeToVM transition(thread);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
     EXPECT(heap->ExternalInWords(Heap::kNew) ==
            (kWeak1ExternalSize + kWeak2ExternalSize) / kWordSize);
     // Collect weakly referenced string, and promote strongly referenced string.
     GCTestHelper::CollectNewSpace(Heap::kIgnoreApiCallbacks);
     GCTestHelper::CollectNewSpace(Heap::kIgnoreApiCallbacks);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT(heap->ExternalInWords(Heap::kNew) == 0);
     EXPECT(heap->ExternalInWords(Heap::kOld) == kWeak2ExternalSize / kWordSize);
   }
   Dart_Isolate isolate = reinterpret_cast<Dart_Isolate>(Isolate::Current());
   Dart_DeleteWeakPersistentHandle(isolate, weak1);
   Dart_DeleteWeakPersistentHandle(isolate, weak2);
   Dart_DeletePersistentHandle(strong_ref);
   {
     TransitionNativeToVM transition(thread);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT(heap->ExternalInWords(Heap::kOld) == 0);
   }
 }
 
 
 TEST_CASE(WeakPersistentHandleExternalAllocationSizeNewspaceGC) {
   Dart_Isolate isolate = reinterpret_cast<Dart_Isolate>(Isolate::Current());
   Heap* heap = Isolate::Current()->heap();
   Dart_WeakPersistentHandle weak1 = NULL;
   // Large enough to exceed any new space limit. Not actually allocated.
@@ skipping 25 matching lines @@
       EXPECT(handle.IsOld());
     }
     EXPECT(heap->ExternalInWords(Heap::kNew) == 0);
     EXPECT(heap->ExternalInWords(Heap::kOld) == kWeak1ExternalSize / kWordSize);
     Dart_ExitScope();
   }
   Dart_DeleteWeakPersistentHandle(isolate, weak1);
   {
     TransitionNativeToVM transition(thread);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
     EXPECT(heap->ExternalInWords(Heap::kOld) == 0);
   }
 }
 
 
 TEST_CASE(WeakPersistentHandleExternalAllocationSizeOldspaceGC) {
   // Check that external allocation in old space can trigger GC.
   Isolate* isolate = Isolate::Current();
   Dart_EnterScope();
   Dart_Handle live = Api::NewHandle(thread, String::New("live", Heap::kOld));
@@ skipping 26 matching lines @@
   Dart_ExitScope();
 }
 
 
 TEST_CASE(WeakPersistentHandleExternalAllocationSizeOddReferents) {
   Heap* heap = Isolate::Current()->heap();
   Dart_WeakPersistentHandle weak1 = NULL;
   static const intptr_t kWeak1ExternalSize = 1 * KB;
   Dart_WeakPersistentHandle weak2 = NULL;
   static const intptr_t kWeak2ExternalSize = 2 * KB;
+  EXPECT_EQ(0, heap->ExternalInWords(Heap::kOld));
   {
     Dart_EnterScope();
     Dart_Handle dart_true = Dart_True();  // VM heap object.
     EXPECT_VALID(dart_true);
     weak1 = Dart_NewWeakPersistentHandle(
-        dart_true, NULL, kWeak1ExternalSize, NopCallback);
+        dart_true, NULL, kWeak1ExternalSize, UnreachedCallback);
     EXPECT_VALID(AsHandle(weak1));
-    Dart_Handle zero = Dart_NewInteger(0);  // Smi.
+    Dart_Handle zero = Dart_False();  // VM heap object.
     EXPECT_VALID(zero);
     weak2 = Dart_NewWeakPersistentHandle(
-        zero, NULL, kWeak2ExternalSize, NopCallback);
+        zero, NULL, kWeak2ExternalSize, UnreachedCallback);
     EXPECT_VALID(AsHandle(weak2));
     // Both should be charged to old space.
     EXPECT(heap->ExternalInWords(Heap::kOld) ==
            (kWeak1ExternalSize + kWeak2ExternalSize) / kWordSize);
     Dart_ExitScope();
   }
   Dart_Isolate isolate = reinterpret_cast<Dart_Isolate>(Isolate::Current());
   Dart_DeleteWeakPersistentHandle(isolate, weak1);
   Dart_DeleteWeakPersistentHandle(isolate, weak2);
+  EXPECT_EQ(0, heap->ExternalInWords(Heap::kOld));
   {
     TransitionNativeToVM transition(thread);
     Isolate::Current()->heap()->CollectGarbage(Heap::kOld);
     EXPECT_EQ(0, heap->ExternalInWords(Heap::kOld));
   }
 }
 
 
 static Dart_WeakPersistentHandle weak1 = NULL;
 static Dart_WeakPersistentHandle weak2 = NULL;
@@ skipping 5739 matching lines @@
     }
 
     Dart_ExitScope();
   }
   EXPECT_EQ(40, peer8);
   EXPECT_EQ(41, peer16);
   EXPECT_EQ(42, canonical_str_peer);
   {
     TransitionNativeToVM transition(thread);
     Isolate::Current()->heap()->CollectAllGarbage();
-    GCTestHelper::WaitForFinalizationTasks();
+    GCTestHelper::WaitForGCTasks();
   }
   EXPECT_EQ(80, peer8);
   EXPECT_EQ(82, peer16);
   EXPECT_EQ(42, canonical_str_peer);  // "*" Symbol is not removed on GC.
 
   FLAG_support_externalizable_strings = saved_flag;
 }
 
 
 TEST_CASE(ExternalizeConstantStrings) {
@@ skipping 1362 matching lines @@
   result = Dart_Invoke(lib,
                        NewString("foozoo"),
                        0,
                        NULL);
   EXPECT(Dart_IsError(result));
 }
 
 #endif  // !PRODUCT
 
 }  // namespace dart