napi: block worker shutdown on in-flight napi_async_work (UAF)

src/jsc/event_loop.rs

@@ -10,7 +10,7 @@
 //! poll deadline). See PORTING.md §Dispatch.
 
 use core::ptr::NonNull;
-use core::sync::atomic::{AtomicI32, AtomicPtr, Ordering};
+use core::sync::atomic::{AtomicI32, AtomicPtr, AtomicU32, Ordering};
 
 use bun_io::{self as Async, Waker};
 use bun_uws as uws;
@@ -88,6 +88,15 @@ pub struct EventLoop {
 
     pub entered_event_loop_count: isize,
     pub concurrent_ref: AtomicI32,
+    /// Count of `WorkPool` jobs scheduled from this VM's JS thread whose
+    /// pool-thread callback has not yet finished its last access to this
+    /// `EventLoop` (typically `enqueue_task_concurrent`). `WebWorker::shutdown`
+    /// spins on this reaching zero before freeing the JSC heap and the
+    /// `VirtualMachine` box that this `EventLoop` is a field of; without that
+    /// barrier the pool thread's callback (and its completion post) are
+    /// use-after-free. Bracket with [`Self::work_pool_task_ref`] /
+    /// [`Self::work_pool_task_unref`].
+    pub work_pool_pending: AtomicU32,
     /// Atomic nullable pointer to the next-due `WTFTimer`.
     ///
     /// Note (§Dispatch): payload is `*mut ()` — the real
@@ -128,6 +137,7 @@ impl Default for EventLoop {
             uws_loop: (),
             entered_event_loop_count: 0,
             concurrent_ref: AtomicI32::new(0),
+            work_pool_pending: AtomicU32::new(0),
             imminent_gc_timer: AtomicPtr::new(core::ptr::null_mut()),
             #[cfg(unix)]
             signal_handler: None,
@@ -989,6 +999,38 @@ impl EventLoop {
         self.wakeup();
     }
 
+    /// JS-thread: call immediately before `WorkPool::schedule` for a task whose
+    /// pool-thread callback will dereference this `EventLoop` / the owning
+    /// `VirtualMachine` / the JSC heap (e.g. to post a completion via
+    /// [`Self::enqueue_task_concurrent`]). Paired with
+    /// [`Self::work_pool_task_unref`] on the pool thread; see
+    /// [`Self::work_pool_pending`].
+    #[inline]
+    pub fn work_pool_task_ref(&self) {
+        self.work_pool_pending.fetch_add(1, Ordering::Relaxed);
+    }
+
+    /// Pool-thread: call as the last `EventLoop`/VM access in the `WorkPool`
+    /// callback (after [`Self::enqueue_task_concurrent`]). The `Release` store
+    /// pairs with [`Self::wait_for_pending_work_pool_tasks`]'s `Acquire` load
+    /// so `WebWorker::shutdown` cannot observe zero until every prior access
+    /// is visible, making the subsequent VM dealloc safe.
+    #[inline]
+    pub fn work_pool_task_unref(&self) {
+        self.work_pool_pending.fetch_sub(1, Ordering::Release);
+    }
+
+    /// Worker-thread shutdown barrier. Spins (yielding) until every
+    /// outstanding [`Self::work_pool_task_ref`] has been matched by
+    /// [`Self::work_pool_task_unref`]. Each pending task is one bounded
+    /// `execute`/compression/IO step, so the wait is bounded; `terminate()`
+    /// already runs user exit handlers before this.
+    pub fn wait_for_pending_work_pool_tasks(&self) {
+        while self.work_pool_pending.load(Ordering::Acquire) > 0 {
+            std::thread::yield_now();
+        }
+    }
+
     pub fn ref_concurrently(&self) {
         let _ = self.concurrent_ref.fetch_add(1, Ordering::SeqCst);
         self.wakeup();

src/jsc/web_worker.rs

@@ -1303,6 +1303,14 @@
             // or observes m_isShuttingDown under m_lock and drops. Idempotent;
             // teardownJSCVM sets it again.
             Bun__JSCTaskScheduler__markShuttingDown(vm.global());
+            // Wait for in-flight WorkPool jobs scheduled from this VM
+            // (napi_async_work today; see `work_pool_pending`). The pool-thread
+            // callback reads this VM's `EventLoop` and JSC-heap-backed buffers
+            // the addon may hold; both are freed below (teardownJSCVM / step-5
+            // dealloc). Runs before the drain so the completion each job posts
+            // is picked up by `__bun_release_task_at_shutdown` while JSC is
+            // still live.
+            vm.event_loop_shared().wait_for_pending_work_pool_tasks();
             // Reclaim queued CppTasks (the per-worker stdio/messaging
             // MessagePort drain tasks that can be in self.tasks mid-tick when
             // terminate() lands, and any Worker dispatchExit close task from a

src/runtime/dispatch.rs

@@ -1331,6 +1331,19 @@
             unsafe { Bun__deleteDeferredWorkTask(task.ptr.cast::<JSCDeferredWorkTask>()) };
             true
         }
+        // A `napi_async_work` completion that reached the queue after the
+        // worker's last tick (the `work_pool_pending` barrier guarantees the
+        // post lands before this drain). Run the addon's `complete` callback
+        // so it can free its per-work native state; JSC is still live here.
+        task_tag::NapiAsyncWork => {
+            // SAFETY: tag identifies pointee; the pool-thread callback ran
+            // (it posted this entry and `work_pool_task_unref()`'d), so the
+            // threadpool no longer holds the embedded `task` field.
+            unsafe {
+                napi_async_work::run_from_js_for_shutdown(task.ptr.cast::<napi_async_work>())
+            };
+            true
+        }
         // Same reclaim `drop_concurrent_cpp_tasks` performs, but for tasks
         // that were already batch-moved into `self.tasks`. Must run before
         // JSC teardown: a Worker `dispatchExit` lambda's `~Ref<Worker>` walks

src/runtime/napi/napi_body.rs

@@ -1758,7 +1758,10 @@
             env: unsafe { NapiEnvRef::clone_from_raw(env.as_mut_ptr()) },
             execute,
             // SAFETY: `event_loop()` is the live JS-thread loop (non-null,
-            // stable address) and outlives every napi_async_work.
+            // stable address). Liveness across a worker `terminate()` is
+            // guaranteed by `schedule()`'s `work_pool_task_ref()`: the worker
+            // shutdown barrier waits for `run()` to `work_pool_task_unref()`
+            // before the VM box (and this `EventLoop`) are freed.
             event_loop: unsafe { bun_ptr::BackRef::from_raw(global.bun_vm().event_loop()) },
             complete,
             data,
@@ -1783,6 +1786,10 @@
         }
         self.scheduled = true;
         self.poll_ref.ref_(bun_io::js_vm_ctx());
+        // Hold the worker-shutdown barrier open until the pool thread has
+        // finished `execute` and posted the completion; matched by
+        // `work_pool_task_unref()` at the end of `run()`.
+        self.event_loop.work_pool_task_ref();
         WorkPool::schedule(&raw mut self.task);
     }
 
@@ -1808,6 +1815,9 @@
                         self.concurrent_task
                             .from(self_ptr, AutoDeinit::ManualDeinit),
                     ));
+                // Last EventLoop/VM access; pairs with `work_pool_task_ref()`
+                // in `schedule()` and releases `WebWorker::shutdown`'s barrier.
+                self.event_loop.work_pool_task_unref();
                 return;
             }
         }
@@ -1819,9 +1829,36 @@
         // queue takes ownership of its `next` link.
         self.event_loop
             .enqueue_task_concurrent(core::ptr::NonNull::from(
                 self.concurrent_task
                     .from(self_ptr, AutoDeinit::ManualDeinit),
             ));
+        // Last EventLoop/VM access; pairs with `work_pool_task_ref()` in
+        // `schedule()` and releases `WebWorker::shutdown`'s barrier.
+        self.event_loop.work_pool_task_unref();
+    }
+
+    /// Shutdown-drain counterpart of [`Self::run_from_js`] for a completion
+    /// that reached the queue after the worker thread stopped ticking (the
+    /// `work_pool_pending` barrier in `WebWorker::shutdown` guarantees the
+    /// post lands before this drain). Runs on the worker's JS thread with the
+    /// JSC heap and VM still live (before `teardownJSCVM`), so it is safe to
+    /// invoke the addon's `complete` callback here. Node.js does the same via
+    /// its env-close `uv_run` drain: `complete` runs with the status `execute`
+    /// left, so the addon can free its per-work native state.
+    ///
+    /// # Safety
+    /// `this` must be the live heap `napi_async_work` whose embedded
+    /// `concurrent_task` was popped from the shutdown drain; the pool thread
+    /// no longer holds it.
+    pub(crate) unsafe fn run_from_js_for_shutdown(this: *mut napi_async_work) {
+        // `complete` may `napi_delete_async_work(self)`, so copy the global
+        // handle out before forming `&mut *this` (matches the normal dispatch
+        // arm, which passes `vm`/`global` from the loop, not from `self`).
+        // SAFETY: see fn contract.
+        let global: GlobalRef = unsafe { (*this).global };
+        // SAFETY: see fn contract; `run_from_js` is documented to tolerate
+        // `self` being freed by the user's `complete`.
+        unsafe { (*this).run_from_js(global.bun_vm().as_mut(), &global) };
     }
 
     pub(crate) fn cancel(&mut self) -> bool {

test/napi/napi-app/binding.gyp

@@ -286,5 +286,16 @@
                 "NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT=1",
             ],
         },
+        {
+            "target_name": "test_async_work_worker_terminate",
+            "sources": ["test_async_work_worker_terminate.c"],
+            "include_dirs": ["<!@(node -p \"require('node-addon-api').include\")"],
+            "libraries": [],
+            "dependencies": ["<!(node -p \"require('node-addon-api').gyp\")"],
+            "defines": [
+                "NAPI_DISABLE_CPP_EXCEPTIONS",
+                "NODE_API_EXPERIMENTAL_NOGC_ENV_OPT_OUT=1",
+            ],
+        },
     ]
 }

test/napi/napi-app/test_async_work_worker_terminate.c

@@ -0,0 +1,113 @@
+#include <node_api.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#ifdef _WIN32
+#include <windows.h>
+static void sleep_ms(unsigned ms) { Sleep(ms); }
+#else
+#include <unistd.h>
+static void sleep_ms(unsigned ms) { usleep(ms * 1000); }
+#endif
+
+#define CHECK(env, call)                                                       \
+  do {                                                                         \
+    napi_status s_ = (call);                                                   \
+    if (s_ != napi_ok) {                                                       \
+      napi_throw_error((env), NULL, #call " failed");                          \
+      return NULL;                                                             \
+    }                                                                          \
+  } while (0)
+
+typedef struct {
+  napi_env env;
+  napi_async_work work;
+  napi_ref buf_ref;
+  napi_ref cb_ref;
+  unsigned char *data;
+  size_t len;
+  unsigned sleep_ms;
+} work_t;
+
+static void exec_cb(napi_env env, void *arg) {
+  work_t *w = (work_t *)arg;
+  sleep_ms(w->sleep_ms);
+  // Touch the ArrayBuffer backing store. Before the fix, worker.terminate()
+  // could free it (via JSC VM teardown running the ArrayBuffer finalizer)
+  // while this callback is still running on the pool thread.
+  if (w->len > 0) {
+    volatile unsigned char first = w->data[0];
+    (void)first;
+    memset(w->data, 0xab, w->len);
+  }
+}
+
+static void done_cb(napi_env env, napi_status status, void *arg) {
+  work_t *w = (work_t *)arg;
+  napi_value cb = NULL, undef = NULL, argv[1];
+  if (w->cb_ref != NULL) {
+    napi_get_reference_value(env, w->cb_ref, &cb);
+  }
+  napi_get_undefined(env, &undef);
+  napi_create_int32(env, (int)status, &argv[0]);
+  if (cb != NULL) {
+    napi_call_function(env, undef, cb, 1, argv, NULL);
+  }
+  napi_delete_reference(env, w->buf_ref);
+  if (w->cb_ref != NULL) napi_delete_reference(env, w->cb_ref);
+  napi_delete_async_work(env, w->work);
+  free(w);
+}
+
+// queueWork(arrayBuffer, ms[, cb]) -> undefined
+static napi_value queue_work(napi_env env, napi_callback_info info) {
+  size_t argc = 3;
+  napi_value argv[3];
+  CHECK(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL));
+  if (argc < 2) {
+    napi_throw_error(env, NULL, "expected (arrayBuffer, ms[, cb])");
+    return NULL;
+  }
+
+  work_t *w = (work_t *)calloc(1, sizeof(*w));
+  w->env = env;
+
+  void *data = NULL;
+  size_t len = 0;
+  CHECK(env, napi_get_arraybuffer_info(env, argv[0], &data, &len));
+  w->data = (unsigned char *)data;
+  w->len = len;
+
+  int32_t ms = 0;
+  CHECK(env, napi_get_value_int32(env, argv[1], &ms));
+  w->sleep_ms = (unsigned)(ms < 0 ? 0 : ms);
+
+  CHECK(env, napi_create_reference(env, argv[0], 1, &w->buf_ref));
+  if (argc >= 3) {
+    napi_valuetype t;
+    CHECK(env, napi_typeof(env, argv[2], &t));
+    if (t == napi_function) {
+      CHECK(env, napi_create_reference(env, argv[2], 1, &w->cb_ref));
+    }
+  }
+
+  napi_value name;
+  CHECK(env, napi_create_string_utf8(env, "test_async_work_worker_terminate",
+                                     NAPI_AUTO_LENGTH, &name));
+  CHECK(env, napi_create_async_work(env, NULL, name, exec_cb, done_cb, w,
+                                    &w->work));
+  CHECK(env, napi_queue_async_work(env, w->work));
+
+  napi_value undef;
+  CHECK(env, napi_get_undefined(env, &undef));
+  return undef;
+}
+
+NAPI_MODULE_INIT() {
+  napi_value fn;
+  CHECK(env, napi_create_function(env, "queueWork", NAPI_AUTO_LENGTH,
+                                  queue_work, NULL, &fn));
+  CHECK(env, napi_set_named_property(env, exports, "queueWork", fn));
+  return exports;
+}

test/napi/napi.test.ts

@@ -518,6 +518,62 @@ describe.concurrent.skipIf(!canBuildNodeAddons())("napi", () => {
       expect(output).toContain("success!");
       expect(output).not.toContain("failure!");
     });
+    // worker.terminate() while execute callbacks are still running on the
+    // thread pool must not free the worker's VirtualMachine (the pool-thread
+    // completion would enqueue into a freed EventLoop) or its JSC heap (the
+    // ArrayBuffer backing store would be finalized while the addon is still
+    // writing it). WebWorker::shutdown now waits for every queued
+    // napi_async_work's pool-thread callback to finish and then runs
+    // complete() on the shutdown drain, matching Node.js. The positive
+    // assertion on stdout catches every crash mode (panic, ASAN abort,
+    // SIGSEGV) without matching on "panic" in stderr; on an unfixed build
+    // the subprocess aborts before printing PASS.
+    it("worker.terminate() with execute callbacks in flight waits for them and does not UAF", async () => {
+      const addon = join(__dirname, "napi-app/build/Debug/test_async_work_worker_terminate.node");
+      const workerSrc = /* js */ `
+        const { parentPort, workerData } = require("node:worker_threads");
+        const addon = require(workerData.addon);
+        const keep = [];
+        for (let i = 0; i < 4; i++) {
+          const ab = new ArrayBuffer(16 << 20);
+          keep.push(ab);
+          addon.queueWork(ab, 300 + i * 50, () => {});
+        }
+        parentPort.postMessage("up");
+        setInterval(() => {}, 1000);
+      `;
+      const script = /* js */ `
+        const { Worker } = require("node:worker_threads");
+        (async () => {
+          for (let r = 0; r < ${isASAN ? 3 : 5}; r++) {
+            const w = new Worker(process.env.WORKER_SRC, {
+              eval: true,
+              workerData: { addon: process.env.ADDON },
+            });
+            let err;
+            w.on("error", e => { err = e; });
+            await new Promise(resolve => {
+              w.once("message", resolve);
+              w.once("exit", resolve);
+            });
+            if (err) throw err;
+            await w.terminate();
+          }
+          console.log("PASS");
+        })().catch(e => {
+          console.error(String(e));
+          process.exit(1);
+        });
+      `;
+      await using proc = spawn({
+        cmd: [bunExe(), "-e", script],
+        env: { ...bunEnv, ADDON: addon, WORKER_SRC: workerSrc },
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "PASS", stderr: "", exitCode: 0 });
+    }, 30_000);
   });
 
   describe("napi_threadsafe_function", () => {