ai slop

src/bun.js/bindings/webcore/Worker.cpp

@@ -71,6 +71,8 @@ WTF_MAKE_TZONE_ALLOCATED_IMPL(Worker);
 
 extern "C" void WebWorker__notifyNeedTermination(
     void* worker);
+extern "C" void WebWorker__destroy(
+    void* worker);
 
 static Lock allWorkersLock;
 static HashMap<ScriptExecutionContextIdentifier, Worker*>& allWorkers() WTF_REQUIRES_LOCK(allWorkersLock)
@@ -228,6 +230,9 @@ Worker::~Worker()
         Locker locker { allWorkersLock };
         allWorkers().remove(m_clientIdentifier);
     }
+    if (impl_) {
+        WebWorker__destroy(impl_);
+    }
     // m_contextProxy.workerObjectDestroyed();
 }
 
@@ -262,7 +267,8 @@ ExceptionOr<void> Worker::postMessage(JSC::JSGlobalObject& state, JSC::JSValue m
 void Worker::terminate()
 {
     // m_contextProxy.terminateWorkerGlobalScope();
-    m_terminationFlags.fetch_or(TerminateRequestedFlag);
+    if (m_terminationFlags.fetch_or(TerminateRequestedFlag))
+        return;
     WebWorker__notifyNeedTermination(impl_);
 }
 

src/bun.js/web_worker.zig

@@ -402,16 +402,24 @@
 
 /// Deinit will clean up vm and everything.
 /// Early deinit may be called from caller thread, but full vm deinit will only be called within worker's thread.
+/// The struct itself is freed separately by the owning C++ `Worker` via `WebWorker__destroy` so
+/// that `WebWorker__notifyNeedTermination` never touches freed memory while JS still holds the
+/// wrapper.
 fn deinit(this: *WebWorker) void {
     log("[{d}] deinit", .{this.execution_context_id});
     this.parent_poll_ref.unrefConcurrently(this.parent);
     bun.default_allocator.free(this.unresolved_specifier);
+    this.unresolved_specifier = "";
     for (this.preloads) |preload| {
         bun.default_allocator.free(preload);
     }
     bun.default_allocator.free(this.preloads);
+    this.preloads = &.{};
+}
+
+export fn WebWorker__destroy(this: *WebWorker) void {
     bun.default_allocator.destroy(this);
 }
 
 fn flushLogs(this: *WebWorker) void {
     jsc.markBinding(@src());
@@ -644,7 +652,13 @@
     }
     var arena = this.arena;
 
+    // Release owned resources before dispatching exit. `WebWorker__dispatchExit` drops the
+    // Zig-held ref on the C++ `Worker`; once the parent processes the close task and GC
+    // collects the wrapper, `~Worker()` will free this struct, so it must not be touched
+    // afterwards.
+    this.deinit();
+
     WebWorker__dispatchExit(globalObject, cpp_worker, exit_code);
     if (loop) |loop_| {
         loop_.internal_loop_data.jsc_vm = null;
     }
@@ -659,8 +673,6 @@
         bun.windows.libuv.Loop.shutdown();
     }
 
-    this.deinit();
-
     if (vm_to_deinit) |vm| {
         vm.deinit(); // NOTE: deinit here isn't implemented, so freeing workers will leak the vm.
     }

test/js/web/workers/worker-terminate-after-exit.test.ts

@@ -0,0 +1,47 @@
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe, isDebug } from "harness";
+
+// The Zig `WebWorker` struct used to be destroyed on the worker thread in
+// `exitAndDeinit`, while `Worker::terminate()` on the main thread could still
+// call `WebWorker__notifyNeedTermination(impl_)` afterwards, reading freed
+// memory. Calling `terminate()` immediately sets `requested_terminate`, so the
+// worker thread takes the fast early-exit path (before its VM is created) and
+// the struct was freed right after the close event was posted. Calling
+// `terminate()` again from the close handler then touched the freed struct.
+test(
+  "Worker.terminate() after the worker thread has exited does not use freed memory",
+  async () => {
+    const code = `
+    for (let i = 0; i < 10; i++) {
+      const w = new Worker("nonexistent-entrypoint-58146");
+      const { promise, resolve } = Promise.withResolvers();
+      w.addEventListener("close", resolve);
+      w.addEventListener("error", () => {});
+      w.terminate();
+      await promise;
+      w.terminate();
+    }
+    Bun.gc(true);
+  `;
+    const concurrency = 5;
+    for (let batch = 0; batch < 4; batch++) {
+      const runs = Array.from({ length: concurrency }, async () => {
+        await using proc = Bun.spawn({
+          cmd: [bunExe(), "-e", code],
+          env: bunEnv,
+          stdout: "pipe",
+          stderr: "pipe",
+        });
+        const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+        return { stderr, exitCode };
+      });
+      for (const { stderr, exitCode } of await Promise.all(runs)) {
+        if (exitCode !== 0) {
+          expect(stderr).toBe("");
+        }
+        expect(exitCode).toBe(0);
+      }
+    }
+  },
+  isDebug ? 60_000 : undefined,
+);