[gardening] Make more robust test for mutator bit stealing mechanism

We also disable the test in hotreload mode, because threads that are
stuck in ffi calls don't participate in hot-reload requests.

This is something we may want to look into. Right now it's conservative in
the sense that if an embedder did a `Dart_LookupClass()` in such a ffi call
and then a reload happened, such a handle would possibly hold pointer to
an old class object and weird things may happen.

This fixes test failures in hot reload / rollback mode of

  `vm/dart/isolates/many_isolates_blocked_at_ffi_test`

and also make it more robust.

TEST=tests/ffi/dl_api_exit_enter_isolate_test

Change-Id: I823d07a6ab04e62e051e4d22ec80cbc3649762a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409100
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Ryan Macnak <[email protected]>

Author: mkustermann

runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc

@@ -890,15 +890,18 @@ DART_EXPORT void ThreadPoolTest_BarrierSync(
     Dart_Isolate (*dart_current_isolate)(),
     void (*dart_enter_isolate)(Dart_Isolate),
     void (*dart_exit_isolate)(),
-    intptr_t num_threads) {
+    intptr_t num_threads,
+    bool exit_and_reenter_isolate) {
   // Guaranteed to be initialized exactly once (no race between multiple
   // threads).
   static std::mutex mutex;
   static std::condition_variable cvar;
   static intptr_t thread_count = 0;
 
   const Dart_Isolate isolate = dart_current_isolate();
-  dart_exit_isolate();
+  if (exit_and_reenter_isolate) {
+    dart_exit_isolate();
+  }
   {
     std::unique_lock<std::mutex> lock(mutex);
     ++thread_count;
@@ -907,7 +910,9 @@ DART_EXPORT void ThreadPoolTest_BarrierSync(
     }
     cvar.notify_all();
   }
-  dart_enter_isolate(isolate);
+  if (exit_and_reenter_isolate) {
+    dart_enter_isolate(isolate);
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1284,9 +1289,11 @@ DART_EXPORT void SetFfiNativeResolverForTest(Dart_Handle url) {
   ENSURE(!Dart_IsError(result));
 }
 
-DART_EXPORT void WaitUntilNThreadsEnterBarrier(intptr_t num_threads) {
+DART_EXPORT void WaitUntilNThreadsEnterBarrier(intptr_t num_threads,
+                                               bool exit_and_reenter_isolate) {
   ThreadPoolTest_BarrierSync(Dart_CurrentIsolate_DL, Dart_EnterIsolate_DL,
-                             Dart_ExitIsolate_DL, num_threads);
+                             Dart_ExitIsolate_DL, num_threads,
+                             exit_and_reenter_isolate);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

runtime/tests/vm/dart/isolates/many_isolates_blocked_at_ffi_test.dart

@@ -28,16 +28,21 @@ typedef Dart_ExitIsolateNFT = Void Function();
 final ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions");
 
 final threadPoolBarrierSync = ffiTestFunctions.lookupFunction<
-    Void Function(
-        Pointer<NativeFunction<Dart_CurrentIsolateNFT>>,
-        Pointer<NativeFunction<Dart_EnterIsolateNFT>>,
-        Pointer<NativeFunction<Dart_ExitIsolateNFT>>,
-        IntPtr),
-    void Function(
-        Pointer<NativeFunction<Dart_CurrentIsolateNFT>>,
-        Pointer<NativeFunction<Dart_EnterIsolateNFT>>,
-        Pointer<NativeFunction<Dart_ExitIsolateNFT>>,
-        int)>('ThreadPoolTest_BarrierSync');
+  Void Function(
+    Pointer<NativeFunction<Dart_CurrentIsolateNFT>>,
+    Pointer<NativeFunction<Dart_EnterIsolateNFT>>,
+    Pointer<NativeFunction<Dart_ExitIsolateNFT>>,
+    IntPtr,
+    Bool,
+  ),
+  void Function(
+    Pointer<NativeFunction<Dart_CurrentIsolateNFT>>,
+    Pointer<NativeFunction<Dart_EnterIsolateNFT>>,
+    Pointer<NativeFunction<Dart_ExitIsolateNFT>>,
+    int,
+    bool,
+  )
+>('ThreadPoolTest_BarrierSync');
 
 final Pointer<NativeFunction<Dart_CurrentIsolateNFT>> dartCurrentIsolate =
     DynamicLibrary.executable().lookup("Dart_CurrentIsolate").cast();
@@ -51,8 +56,14 @@ class Worker extends RingElement {
   Worker(this.id);
 
   Future run(dynamic _, dynamic _2) async {
+    const bool exitAndReenterIsolate = true;
     threadPoolBarrierSync(
-        dartCurrentIsolate, dartEnterIsolate, dartExitIsolate, threadCount);
+      dartCurrentIsolate,
+      dartEnterIsolate,
+      dartExitIsolate,
+      threadCount,
+      exitAndReenterIsolate,
+    );
     return id;
   }
 }

runtime/tests/vm/dart/isolates/thread_pool_test.dart

@@ -56,7 +56,6 @@ dart/kernel_determinism_test: SkipSlow
 dart/minimal_kernel_test: SkipSlow # gen_kernel is too slow with optimization_counter_threshold
 
 [ $compiler == app_jitk ]
-dart/isolates/many_isolates_blocked_at_ffi_test: Skip # App snapshot creation needs regular isolate shutdown
 dart/isolates/many_isolates_blocked_at_process_run_sync_test: Skip # App snapshot creation needs regular isolate shutdown
 dart/isolates/many_isolates_blocked_at_sleep_test: Skip # App snapshot creation needs regular isolate shutdown
 dart/snapshot_version_test: RuntimeError

runtime/tests/vm/vm.status

@@ -16,18 +16,32 @@ final initializeApi = ffiTestFunctions.lookupFunction<
   int Function(Pointer<Void>)
 >("InitDartApiDL");
 final enterBarrier = ffiTestFunctions
-    .lookupFunction<Void Function(IntPtr), void Function(int)>(
+    .lookupFunction<Void Function(IntPtr, Bool), void Function(int, bool)>(
       "WaitUntilNThreadsEnterBarrier",
     );
 
 main() async {
+  initializeApi(NativeApi.initializeApiDLData);
+
   const threadBarrierCount = 30;
 
-  initializeApi(NativeApi.initializeApiDLData);
+  // The threads may explicitly decrease the mutator count by using
+  //   * Dart_ExitIsolate()
+  //   * block and
+  //   * Dart_EnterIsolate()
+  await testNativeBarrier(threadBarrierCount, true);
+
+  // The threads may not explicitly exit the isolate but the VM may implicitly
+  // (when entering an isolate for execution) kick other threads out of the
+  // mutator count (by making them implicitly go to slow path when trying to
+  // leave a safepoint).
+  await testNativeBarrier(threadBarrierCount, false);
+}
 
+Future testNativeBarrier(int count, bool exitAndReenterIsolate) async {
   final all = <Future>[];
-  for (int i = 0; i < threadBarrierCount; ++i) {
-    all.add(Isolate.run(() => enterBarrier(threadBarrierCount)));
+  for (int i = 0; i < count; ++i) {
+    all.add(Isolate.run(() => enterBarrier(count, true)));
   }
   await Future.wait(all);
 }