Run the new test

src/bun.js/bindings/JSGlobalObject.zig

@@ -232,7 +232,7 @@ pub const JSGlobalObject = opaque {
             defer buf.deinit();
             var writer = buf.writer();
             writer.print(fmt, args) catch
-            // if an exception occurs in the middle of formatting the error message, it's better to just return the formatting string than an error about an error
+                // if an exception occurs in the middle of formatting the error message, it's better to just return the formatting string than an error about an error
                 return ZigString.static(fmt).toErrorInstance(this);
 
             // Ensure we clone it.
@@ -486,20 +486,58 @@ pub const JSGlobalObject = opaque {
     }
 
     pub fn bunVM(this: *JSGlobalObject) *JSC.VirtualMachine {
-        if (comptime bun.Environment.allow_assert) {
-            // if this fails
-            // you most likely need to run
-            //   make clean-jsc-bindings
-            //   make bindings -j10
-            const assertion = this.bunVMUnsafe() == @as(*anyopaque, @ptrCast(JSC.VirtualMachine.get()));
-            bun.assert(assertion);
+        const vm_holder_vm = JSC.VirtualMachine.VMHolder.vm orelse
+            @panic("internal assertion failure: bunVM() called on thread with no VM");
+        const global_object_vm = this.bunVMConcurrently();
+        if (bun.Environment.allow_assert) {
+            bun.assertf(global_object_vm == vm_holder_vm,
+                \\Bun VM for global object {x} mismatched:
+                \\{x} in VMHolder
+                \\{x} from JSC__JSGlobalObject__bunVM
+                \\either there is an inconsistency with the bindings, or bunVM() was called on the wrong thread.
+            , .{
+                @intFromPtr(this),
+                @intFromPtr(vm_holder_vm),
+                @intFromPtr(global_object_vm),
+            });
+        }
+
+        return global_object_vm;
+    }
+
+    pub const TryBunVMResult = union(enum) {
+        /// Running on the main thread executing JS code for this virtual machine.
+        main_thread: *JSC.VirtualMachine,
+        /// Running on a different thread which does not have a JS virtual machine. Only concurrent
+        /// APIs may be used.
+        other_thread: *JSC.VirtualMachine,
+    };
+
+    /// Get the Bun VM for this global object as well as information about what thread you are on
+    /// with respect to that VM. `.main` means you are on the main thread that runs JS code for that
+    /// VM, and `.other` means you are on any other thread.
+    ///
+    /// Must not be called from any other VM's main thread (i.e. a Worker), but this may be called
+    /// from any other thread that doesn't have a VM (e.g. a JSC heap thread or a Bun work pool
+    /// thread)
+    pub fn tryBunVM(this: *JSGlobalObject) TryBunVMResult {
+        const global_object_vm = this.bunVMConcurrently();
+
+        if (JSC.VirtualMachine.VMHolder.vm) |this_thread_vm| {
+            bun.assertf(
+                global_object_vm == this_thread_vm,
+                "tryBunVM called from a different VM's main thread",
+                .{},
+            );
+            return .{ .main_thread = global_object_vm };
+        } else {
+            return .{ .other_thread = global_object_vm };
         }
-        return @as(*JSC.VirtualMachine, @ptrCast(@alignCast(this.bunVMUnsafe())));
     }
 
     /// We can't do the threadlocal check when queued from another thread
     pub fn bunVMConcurrently(this: *JSGlobalObject) *JSC.VirtualMachine {
-        return @as(*JSC.VirtualMachine, @ptrCast(@alignCast(this.bunVMUnsafe())));
+        return @ptrCast(@alignCast(this.bunVMUnsafe()));
     }
 
     extern fn JSC__JSGlobalObject__handleRejectedPromises(*JSGlobalObject) void;

src/napi/napi.zig

@@ -2137,12 +2137,20 @@ pub const NapiFinalizerTask = struct {
     }
 
     pub fn schedule(this: *NapiFinalizerTask) void {
-        const vm = this.finalizer.env.?.toJS().bunVM();
-        if (vm.isShuttingDown()) {
-            // Immediate tasks won't run, so we run this as a cleanup hook instead
-            vm.rareData().pushCleanupHook(vm.global, this, runAsCleanupHook);
-        } else {
-            this.finalizer.env.?.toJS().bunVM().event_loop.enqueueImmediateTask(JSC.Task.init(this));
+        const globalThis = this.finalizer.env.?.toJS();
+        switch (globalThis.tryBunVM()) {
+            .main_thread => |vm| {
+                if (vm.isShuttingDown()) {
+                    // Immediate tasks won't run, so we run this as a cleanup hook instead
+                    vm.rareData().pushCleanupHook(vm.global, this, runAsCleanupHook);
+                } else {
+                    vm.event_loop.enqueueImmediateTask(JSC.Task.init(this));
+                }
+            },
+            .other_thread => |vm| {
+                // TODO(@heimskr): do we need to handle the case where the vm is shutting down?
+                vm.eventLoop().enqueueTaskConcurrent(JSC.ConcurrentTask.create(JSC.Task.init(this)));
+            },
         }
     }
 

test/napi/napi-app/async_finalize_addon.c

@@ -1,72 +0,0 @@
-// This is a separate addon because the main one is built with
-// NAPI_VERSION_EXPERIMENTAL, which makes finalizers run synchronously during GC
-// and requires node_api_post_finalizer to run functions that could affect JS
-// engine state. This module's purpose is to call napi_delete_reference directly
-// during a finalizer -- not during a callback scheduled with
-// node_api_post_finalizer -- so it cannot use NAPI_VERSION_EXPERIMENTAL.
-
-#include <assert.h>
-#include <js_native_api.h>
-#include <node_api.h>
-#include <stdio.h>
-#include <stdlib.h>
-
-// "we have static_assert at home" - MSVC
-char assertion[NAPI_VERSION == 8 ? 1 : -1];
-
-#define NODE_API_CALL_CUSTOM_RETURN(env, call, retval)                         \
-  do {                                                                         \
-    napi_status status = (call);                                               \
-    if (status != napi_ok) {                                                   \
-      const napi_extended_error_info *error_info = NULL;                       \
-      napi_get_last_error_info((env), &error_info);                            \
-      const char *err_message = error_info->error_message;                     \
-      bool is_pending;                                                         \
-      napi_is_exception_pending((env), &is_pending);                           \
-      /* If an exception is already pending, don't rethrow it */               \
-      if (!is_pending) {                                                       \
-        const char *message =                                                  \
-            (err_message == NULL) ? "empty error message" : err_message;       \
-        napi_throw_error((env), NULL, message);                                \
-      }                                                                        \
-      return retval;                                                           \
-    }                                                                          \
-  } while (0)
-
-#define NODE_API_CALL(env, call) NODE_API_CALL_CUSTOM_RETURN(env, call, NULL)
-#define NODE_API_CALL_RETURN_VOID(env, call)                                   \
-  NODE_API_CALL_CUSTOM_RETURN(env, call, )
-
-typedef struct {
-  napi_ref ref;
-} RefHolder;
-
-static void finalizer(napi_env env, void *data, void *hint) {
-  printf("finalizer\n");
-  (void)hint;
-  RefHolder *ref_holder = (RefHolder *)data;
-  NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, ref_holder->ref));
-  free(ref_holder);
-}
-
-static napi_value create_ref(napi_env env, napi_callback_info info) {
-  (void)info;
-  napi_value object;
-  NODE_API_CALL(env, napi_create_object(env, &object));
-  RefHolder *ref_holder = calloc(1, sizeof *ref_holder);
-  NODE_API_CALL(env, napi_wrap(env, object, ref_holder, finalizer, NULL,
-                               &ref_holder->ref));
-  napi_value undefined;
-  NODE_API_CALL(env, napi_get_undefined(env, &undefined));
-  return undefined;
-}
-
-/* napi_value */ NAPI_MODULE_INIT(/* napi_env env, napi_value exports */) {
-  napi_value create_ref_function;
-  NODE_API_CALL(env,
-                napi_create_function(env, "create_ref", NAPI_AUTO_LENGTH,
-                                     create_ref, NULL, &create_ref_function));
-  NODE_API_CALL(env, napi_set_named_property(env, exports, "create_ref",
-                                             create_ref_function));
-  return exports;
-}

test/napi/napi-app/async_finalize_addon.cpp

@@ -0,0 +1,119 @@
+// This is a separate addon because the main one is built with
+// NAPI_VERSION_EXPERIMENTAL, which makes finalizers run synchronously during GC
+// and requires node_api_post_finalizer to run functions that could affect JS
+// engine state. This module's purpose is to call napi_delete_reference directly
+// during a finalizer -- not during a callback scheduled with
+// node_api_post_finalizer -- so it cannot use NAPI_VERSION_EXPERIMENTAL.
+
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <js_native_api.h>
+#include <node_api.h>
+#include <thread>
+
+static_assert(NAPI_VERSION == 8,
+              "this module must be built with Node-API version 8");
+
+static std::thread::id js_thread_id;
+
+#define NODE_API_CALL_CUSTOM_RETURN(env, call, retval)                         \
+  do {                                                                         \
+    napi_status status = (call);                                               \
+    if (status != napi_ok) {                                                   \
+      const napi_extended_error_info *error_info = NULL;                       \
+      napi_get_last_error_info((env), &error_info);                            \
+      const char *err_message = error_info->error_message;                     \
+      bool is_pending;                                                         \
+      napi_is_exception_pending((env), &is_pending);                           \
+      /* If an exception is already pending, don't rethrow it */               \
+      if (!is_pending) {                                                       \
+        const char *message =                                                  \
+            (err_message == NULL) ? "empty error message" : err_message;       \
+        napi_throw_error((env), NULL, message);                                \
+      }                                                                        \
+      return retval;                                                           \
+    }                                                                          \
+  } while (0)
+
+#define NODE_API_CALL(env, call) NODE_API_CALL_CUSTOM_RETURN(env, call, NULL)
+#define NODE_API_CALL_RETURN_VOID(env, call)                                   \
+  NODE_API_CALL_CUSTOM_RETURN(env, call, )
+
+static void finalizer(napi_env env, void *data, void *hint) {
+  printf("finalizer\n");
+  (void)hint;
+  napi_ref *ref_holder = reinterpret_cast<napi_ref *>(data);
+  NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, *ref_holder));
+  delete ref_holder;
+}
+
+static napi_value create_ref(napi_env env, napi_callback_info info) {
+  (void)info;
+  napi_value object;
+  NODE_API_CALL(env, napi_create_object(env, &object));
+  napi_ref *ref_holder = new napi_ref;
+  NODE_API_CALL(
+      env, napi_wrap(env, object, ref_holder, finalizer, NULL, ref_holder));
+  napi_value undefined;
+  NODE_API_CALL(env, napi_get_undefined(env, &undefined));
+  return undefined;
+}
+
+static int buffer_finalize_count = 0;
+
+static void buffer_finalizer(napi_env env, void *data, void *hint) {
+  (void)hint;
+  if (std::this_thread::get_id() == js_thread_id) {
+    printf("buffer_finalizer run from js thread\n");
+  } else {
+    printf("buffer_finalizer run from another thread\n");
+  }
+  free(data);
+  buffer_finalize_count++;
+}
+
+static napi_value create_buffer(napi_env env, napi_callback_info info) {
+  (void)info;
+  static const size_t len = 1000000;
+  void *data = malloc(len);
+  memset(data, 5, len);
+  napi_value buf;
+  // JavaScriptCore often runs external ArrayBuffer finalizers off the main
+  // thread. In this case, we need to concurrently post a task to the main
+  // thread to invoke the finalizer.
+  NODE_API_CALL(env, napi_create_external_arraybuffer(
+                         env, data, len, buffer_finalizer, NULL, &buf));
+  return buf;
+}
+
+static napi_value get_buffer_finalize_count(napi_env env,
+                                            napi_callback_info info) {
+  (void)info;
+  napi_value count;
+  NODE_API_CALL(env, napi_create_int32(env, buffer_finalize_count, &count));
+  return count;
+}
+
+/* napi_value */ NAPI_MODULE_INIT(/* napi_env env, napi_value exports */) {
+  js_thread_id = std::this_thread::get_id();
+
+  napi_value js_function;
+  NODE_API_CALL(env, napi_create_function(env, "create_ref", NAPI_AUTO_LENGTH,
+                                          create_ref, NULL, &js_function));
+  NODE_API_CALL(
+      env, napi_set_named_property(env, exports, "create_ref", js_function));
+  NODE_API_CALL(env,
+                napi_create_function(env, "create_buffer", NAPI_AUTO_LENGTH,
+                                     create_buffer, NULL, &js_function));
+  NODE_API_CALL(
+      env, napi_set_named_property(env, exports, "create_buffer", js_function));
+  NODE_API_CALL(env, napi_create_function(
+                         env, "get_buffer_finalize_count", NAPI_AUTO_LENGTH,
+                         get_buffer_finalize_count, NULL, &js_function));
+  NODE_API_CALL(env, napi_set_named_property(env, exports,
+                                             "get_buffer_finalize_count",
+                                             js_function));
+
+  return exports;
+}

test/napi/napi-app/binding.gyp

@@ -81,7 +81,7 @@
         },
         {
             "target_name": "async_finalize_addon",
-            "sources": ["async_finalize_addon.c"],
+            "sources": ["async_finalize_addon.cpp"],
             "include_dirs": ["<!@(node -p \"require('node-addon-api').include\")"],
             "libraries": [],
             "dependencies": ["<!(node -p \"require('node-addon-api').gyp\")"],

test/napi/napi-app/module.js

@@ -572,6 +572,20 @@ nativeTests.test_ref_deleted_in_async_finalize = () => {
   asyncFinalizeAddon.create_ref();
 };
 
+nativeTests.test_external_buffer_finalizer = async () => {
+  const n = 10;
+  for (let i = 0; i < n; i++) {
+    const arraybuffer = asyncFinalizeAddon.create_buffer();
+    const view = new Uint8Array(arraybuffer);
+    for (const byte of view) {
+      assert(byte == 5);
+    }
+    // give GC a chance to do some work on another thread
+    await new Promise(resolve => setTimeout(resolve, 1));
+  }
+  await gcUntil(() => asyncFinalizeAddon.get_buffer_finalize_count() == n);
+};
+
 nativeTests.test_create_bigint_words = () => {
   console.log(nativeTests.create_weird_bigints());
 };

test/napi/napi.test.ts

@@ -342,7 +342,11 @@ describe("napi", () => {
     });
   });
 
-  // TODO(@190n) test allocating in a finalizer from a napi module with the right version
+  describe("finalizers", () => {
+    it("runs asynchronously on the JS thread", () => {
+      checkSameOutput("test_external_buffer_finalizer", []);
+    });
+  });
 
   describe("napi_wrap", () => {
     it("accepts the right kinds of values", () => {