From 3be538106027f9018634fb34306902e2fe6c4c97 Mon Sep 17 00:00:00 2001
From: Aapo Alasuutari <aapo.alasuutari@gmail.com>
Date: Wed, 28 Jun 2023 23:41:59 +0300
Subject: [PATCH] fix(test_ffi): thread_safe_callback is flaky (#19640)

Attempts to fix the thread_safe_callback flakiness. It's unclear what
the flake is about, the exit code is apparently `C0000005` or
`ACCESS_VIOLATION`, pointing to an issue with memory access. My only
guess is that maybe dropping the `Option<extern "C" fn ()>` is somehow
checking the validity of the function pointer and since the function has
been dropped, the pointer is no longer valid and sometimes points to
memory that should not be accessed.

So now the will explicitly drop the functions before they get
deallocated. If this doesn't fix the flake then something beyond my
understanding is wrong.
---
 test_ffi/tests/integration_tests.rs       | 2 ++
 test_ffi/tests/thread_safe_test.js        | 5 +++++
 test_ffi/tests/thread_safe_test_worker.js | 2 +-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/test_ffi/tests/integration_tests.rs b/test_ffi/tests/integration_tests.rs
index e850a174a3..a4f233e45d 100644
--- a/test_ffi/tests/integration_tests.rs
+++ b/test_ffi/tests/integration_tests.rs
@@ -176,8 +176,10 @@ fn thread_safe_callback() {
   let expected = "\
     Callback on main thread\n\
     Callback on worker thread\n\
+    STORED_FUNCTION cleared\n\
     Calling callback, isolate should stay asleep until callback is called\n\
     Callback being called\n\
+    STORED_FUNCTION cleared\n\
     Isolate should now exit\n";
   assert_eq!(stdout, expected);
   assert_eq!(stderr, "");
diff --git a/test_ffi/tests/thread_safe_test.js b/test_ffi/tests/thread_safe_test.js
index 62c78279dd..41ab803be8 100644
--- a/test_ffi/tests/thread_safe_test.js
+++ b/test_ffi/tests/thread_safe_test.js
@@ -71,6 +71,8 @@ dylib.symbols.call_stored_function();
 
 // Unref both main and worker thread callbacks and terminate the worker: Note, the stored function pointer in lib is now dangling.
 
+dylib.symbols.store_function(null);
+
 mainThreadCallback.unref();
 await sendWorkerMessage("unref");
 worker.terminate();
@@ -90,6 +92,9 @@ cleanupCallback.ref();
 
 function cleanup() {
   cleanupCallback.unref();
+  dylib.symbols.store_function(null);
+  mainThreadCallback.close();
+  cleanupCallback.close();
   console.log("Isolate should now exit");
 }
 
diff --git a/test_ffi/tests/thread_safe_test_worker.js b/test_ffi/tests/thread_safe_test_worker.js
index a87a975569..fb3365bf81 100644
--- a/test_ffi/tests/thread_safe_test_worker.js
+++ b/test_ffi/tests/thread_safe_test_worker.js
@@ -35,7 +35,7 @@ self.addEventListener("message", ({ data }) => {
   } else if (data === "call") {
     dylib.symbols.call_stored_function();
   } else if (data === "unref") {
-    callback.unref();
+    callback.close();
   }
   self.postMessage("done");
 });