[test] Avoid data race checking for isolate cleanup callback.

Remove manual MSAN unpoisoning that should happen automatically for Pointer/TypedData writes.

TEST=tsan, msan
Bug: #61503
Bug: #61772
Change-Id: I7c3af7ad769f36a869de7f4d8125d0ced47c9ec7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/458069
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc

@@ -331,21 +331,48 @@ DART_EXPORT void RestoreSIGPIPEHandler() {
 #endif
 }
 
-DART_EXPORT void IGH_MsanUnpoison(void* start, intptr_t length) {
-  MSAN_UNPOISON(start, length);
+struct IGH_Peer {
+  bool shutdown = false;
+  bool cleanup = false;
+  std::mutex mutex;
+  std::condition_variable cvar;
+};
+
+DART_EXPORT void* IGH_CreatePeer() {
+  return new IGH_Peer();
+}
+
+DART_EXPORT void IGH_CheckPeerShutdown(void* peer_in) {
+  IGH_Peer* peer = reinterpret_cast<IGH_Peer*>(peer_in);
+  ENSURE(peer->shutdown);
+
+  {
+    std::unique_lock<std::mutex> lock(peer->mutex);
+    while (!peer->cleanup) {
+      peer->cvar.wait(lock);
+    }
+  }
+  ENSURE(peer->cleanup);
+
+  delete peer;
 }
 
 DART_EXPORT Dart_Isolate IGH_CreateIsolate(const char* name, void* peer) {
   struct Helper {
     static void ShutdownCallback(void* ig_data, void* isolate_data) {
-      char* string = reinterpret_cast<char*>(isolate_data);
-      ENSURE(string[0] == 'a');
-      string[0] = 'x';
+      IGH_Peer* peer = reinterpret_cast<IGH_Peer*>(isolate_data);
+      ENSURE(!peer->shutdown);
+      peer->shutdown = true;
     }
     static void CleanupCallback(void* ig_data, void* isolate_data) {
-      char* string = reinterpret_cast<char*>(isolate_data);
-      ENSURE(string[2] == 'c');
-      string[2] = 'z';
+      IGH_Peer* peer = reinterpret_cast<IGH_Peer*>(isolate_data);
+      ENSURE(!peer->cleanup);
+
+      {
+        std::unique_lock<std::mutex> lock(peer->mutex);
+        peer->cleanup = true;
+        peer->cvar.notify_one();
+      }
     }
   };
 

runtime/tests/vm/dart/isolates/dart_api_create_lightweight_isolate_test.dart

@@ -27,11 +27,16 @@ final class Isolate extends Opaque {}
 abstract class FfiBindings {
   static final ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions");
 
-  static final IGH_MsanUnpoison = ffiTestFunctions
+  static final IGH_CreatePeer = ffiTestFunctions
+      .lookupFunction<Pointer<Void> Function(), Pointer<Void> Function()>(
+        'IGH_CreatePeer',
+      );
+
+  static final IGH_CheckPeerShutdown = ffiTestFunctions
       .lookupFunction<
-        Pointer<Isolate> Function(Pointer<Void>, IntPtr),
-        Pointer<Isolate> Function(Pointer<Void>, int)
-      >('IGH_MsanUnpoison');
+        Void Function(Pointer<Void>),
+        void Function(Pointer<Void>)
+      >('IGH_CheckPeerShutdown');
 
   static final IGH_CreateIsolate = ffiTestFunctions
       .lookupFunction<
@@ -82,7 +87,6 @@ abstract class FfiBindings {
     Pointer<Void> peer,
   ) {
     final cname = name.toNativeUtf8();
-    IGH_MsanUnpoison(cname.cast(), name.length + 10);
     try {
       final isolate = IGH_CreateIsolate(cname, peer);
       Expect.isTrue(isolate.address != 0);
@@ -105,9 +109,7 @@ abstract class FfiBindings {
     );
     final dartScript = dartScriptUri.toString();
     final libraryUri = dartScript.toNativeUtf8();
-    IGH_MsanUnpoison(libraryUri.cast(), dartScript.length + 1);
     final functionName = name.toNativeUtf8();
-    IGH_MsanUnpoison(functionName.cast(), name.length + 1);
 
     IGH_StartIsolate(
       isolate,
@@ -137,23 +139,14 @@ void scheduleAsyncInvocation(void fun()) {
 }
 
 Future withPeerPointer(fun(Pointer<Void> peer)) async {
-  final Pointer<Void> peer = 'abc'.toNativeUtf8().cast();
-  FfiBindings.IGH_MsanUnpoison(peer.cast(), 'abc'.length + 1);
+  final Pointer<Void> peer = FfiBindings.IGH_CreatePeer();
   try {
     await fun(peer);
   } catch (e, s) {
     print('Exception: $e\nStack:$s');
     rethrow;
   } finally {
-    // The shutdown callback is called before the exit listeners are notified, so
-    // we can validate that a->x has been changed.
-    Expect.isTrue(peer.cast<Utf8>().toDartString().startsWith('xb'));
-
-    // The cleanup callback is called after notifying exit listeners. So we
-    // wait a little here to ensure the write of the callback has arrived.
-    await Future.delayed(const Duration(milliseconds: 100));
-    Expect.equals('xbz', peer.cast<Utf8>().toDartString());
-    calloc.free(peer);
+    FfiBindings.IGH_CheckPeerShutdown(peer);
   }
 }