JSPropertyIterator: drop Ref<VM>; free on trap-in-window so worker ~VM runs

src/jsc/JSPropertyIterator.rs

@@ -260,17 +260,29 @@
         own_properties_only: bool,
         only_non_index_properties: bool,
     ) -> JsResult<Option<NonNull<JSPropertyIteratorImpl>>> {
-        // may return null without an exception
-        let raw = from_js_host_call_generic(global_object, || {
-            Bun__JSPropertyIterator__create(
+        // Own the returned allocation before the post-call trap check: a
+        // termination request set between the FFI's RETURN_IF_EXCEPTION and
+        // ours would otherwise drop the raw pointer (no-op) and leak it.
+        let mut raw: *mut JSPropertyIteratorImpl = core::ptr::null_mut();
+        let check = from_js_host_call_generic(global_object, || {
+            raw = Bun__JSPropertyIterator__create(
                 global_object,
                 JSValue::from_cell(object),
                 count,
                 own_properties_only,
                 only_non_index_properties,
-            )
-        })?;
-        Ok(NonNull::new(raw))
+            );
+        });
+        let impl_ = NonNull::new(raw);
+        if let Err(e) = check {
+            if let Some(p) = impl_ {
+                // SAFETY: `p` came from Bun__JSPropertyIterator__create above
+                // and has not been freed.
+                unsafe { Bun__JSPropertyIterator__deinit(p.as_ptr()) };
+            }
+            return Err(e);
+        }
+        Ok(impl_)
     }
 
     pub fn get_name_and_value(

src/jsc/bindings/JSPropertyIterator.cpp

@@ -26,7 +26,9 @@ class JSPropertyIterator {
     }
 
     RefPtr<JSC::PropertyNameArray> properties;
-    Ref<JSC::VM> vm;
+    // Raw reference: the Rust `JSPropertyIterator` owner is stack-scoped with a
+    // lifetime tied to its `&JSGlobalObject`, so the VM always outlives this.
+    JSC::VM& vm;
     bool isSpecialProxy = false;
     static JSPropertyIterator* create(JSC::VM& vm, RefPtr<JSC::PropertyNameArray> data)
     {
@@ -163,7 +165,7 @@ extern "C" EncodedJSValue Bun__JSPropertyIterator__getNameAndValueNonObservable(
         RELEASE_AND_RETURN(scope, getOwnProxyObject(iter, object, prop, propertyName));
     }
 
-    PropertySlot slot(object, PropertySlot::InternalMethodType::VMInquiry, vm.ptr());
+    PropertySlot slot(object, PropertySlot::InternalMethodType::VMInquiry, &vm);
     auto has = object->getNonIndexPropertySlot(globalObject, prop, slot);
     RETURN_IF_EXCEPTION(scope, {});
     if (!has) {

test/js/node/worker_threads/worker_threads.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it, setDefaultTimeout, test } from "bun:test";
-import { bunEnv, bunExe, isDebug, tmpdirSync } from "harness";
+import { bunEnv, bunExe, isASAN, isDebug, isWindows, tmpdirSync } from "harness";
 import { once } from "node:events";
 import fs from "node:fs";
 import { join, relative, resolve } from "node:path";
@@ -1756,3 +1756,38 @@ test("the SHARE_ENV founding thread's process.env stays live after the swap", as
   expect(stdout.trim()).toBe("yes,unset");
   expect(exitCode).toBe(0);
 });
+
+// JSPropertyIterator held a Ref<JSC::VM>. When a worker was terminated in the
+// handful of instructions between the C++ create()'s own exception check and
+// the Rust caller's post-call trap check, the returned raw pointer was dropped
+// (a no-op) and the iterator leaked that VM ref, so the worker's deref() left
+// the refcount at 1 and ~VM (and every cell finalizer) was skipped. The race
+// is narrow; http2's request() path walks headers via JSPropertyIterator on
+// every tick, so a chain of terminate()-mid-dispatch workers hits it reliably.
+test.skipIf(!isASAN || isWindows)(
+  "terminate() during native property iteration still runs the worker VM's finalizers",
+  async () => {
+    const env = {
+      ...bunEnv,
+      BUN_DESTRUCT_VM_ON_EXIT: "1",
+      ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"),
+      LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dirname, "../../../leaksan.supp")}`,
+    };
+    // Six independent processes so a regression fails fast at the first LSan
+    // sweep instead of after one ~2-minute 252-worker run.
+    for (let i = 0; i < 6; i++) {
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), join(import.meta.dirname, "worker-terminate-propiter-parent-fixture.js")],
+        env,
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect(stderr).not.toContain("LeakSanitizer");
+      expect(stderr).not.toMatch(/h2::connection|h2_frame_parser|JSPropertyIterator|PropertyNameArray/);
+      expect(stdout).toBe("");
+      expect(exitCode).toBe(0);
+    }
+  },
+  180_000,
+);