From 5517531416e0d0d16bc682cbb7f818d8b3d528f9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:22:19 +0000 Subject: [PATCH 1/3] dns: do not resolve with an empty value when worker.terminate() races a lookup completion When a parent calls worker.terminate() while the worker thread is draining getaddrinfo completions, the JSC termination flag can fire in the middle of the result-to-JSArray conversion. The conversion then throws, the caller falls back to JSValue::ZERO, and the DNS on_complete path passed that empty value straight into JSC__JSPromise__resolve, tripping ASSERT(!target.isEmpty()) in debug and a near-null (0x5) SIGSEGV in release. Guard the four DNS on_complete sinks (DNSLookup::on_complete_with_array, CAresLookup::on_complete, CAresReverse::on_complete, CAresNameInfo::on_complete) so an empty result drops the promise Strong and cleans up instead of calling resolve. Also fix the one drain_pending_host re-conversion site that would have panicked on an Err from result_any_to_js, and add a GetAddrInfoRequestTask release arm so queued-but-never-dispatched completions free their JSPromiseStrong handles while the JSC VM is still alive instead of being stranded past worker VM dealloc. --- src/runtime/dispatch.rs | 18 +++++ src/runtime/dns_jsc/dns.rs | 64 ++++++++++++++++-- .../workers/worker-terminate-lifetime.test.ts | 65 ++++++++++++++++++- 3 files changed, 140 insertions(+), 7 deletions(-) diff --git a/src/runtime/dispatch.rs b/src/runtime/dispatch.rs index 685c1006bcc1..9da5097ad7e5 100644 --- a/src/runtime/dispatch.rs +++ b/src/runtime/dispatch.rs @@ -1213,6 +1213,24 @@ pub(crate) fn __bun_release_task_at_shutdown(task: bun_event_loop::Task) -> bool unsafe { Bun__deleteDeferredWorkTask(task.ptr.cast::()) }; true } + // A libc `getaddrinfo` completion the work pool posted back but + // `tick()` never dispatched. Free the `WorkTask`, the + // `GetAddrInfoRequest`, and the `DNSLookup` chain so their + // `JSPromiseStrong` handles drop while the JSC VM is still live; + // re-queuing would strand them past worker VM dealloc. + #[cfg(not(windows))] + task_tag::GetAddrInfoRequestTask => { + let wt = task.ptr.cast::(); + // SAFETY: tag identifies pointee; `ctx` is the heap request + // `WorkTask::create_on_js_thread` stored. + let ctx = unsafe { (*wt).ctx }; + // SAFETY: `ctx` was `heap::into_raw`'d in `GetAddrInfoRequest::init` + // and the work-pool `run` has completed (it posted this entry). + unsafe { crate::dns_jsc::GetAddrInfoRequest::release_for_shutdown(ctx) }; + // SAFETY: paired with `create_on_js_thread` heap::alloc. + unsafe { get_addr_info_request::Task::destroy(wt) }; + 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` walks diff --git a/src/runtime/dns_jsc/dns.rs b/src/runtime/dns_jsc/dns.rs index 88533da4b7ed..de67521eed45 100644 --- a/src/runtime/dns_jsc/dns.rs +++ b/src/runtime/dns_jsc/dns.rs @@ -942,7 +942,12 @@ impl CAresNameInfo { let mut promise = unsafe { core::mem::take(&mut (*this).promise) }; // SAFETY: see fn contract — `this` is a live node. let global_this = unsafe { (*this).global_this() }; - let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + // An empty `result` means the caller's JS-result conversion threw + // (JSC termination during worker teardown). Resolving with it would + // trip `ASSERT(!target.isEmpty())` in `JSC__JSPromise__resolve`. + if !result.is_empty() { + let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + } // SAFETY: see fn contract. unsafe { Self::destroy(this) }; } @@ -1561,6 +1566,37 @@ impl GetAddrInfoRequest { } } + /// Shutdown-time reclaim for a queued-but-never-dispatched completion + /// (`__bun_release_task_at_shutdown`'s `GetAddrInfoRequestTask` arm). + /// Frees the request box and every `DNSLookup` in its chain so their + /// `JSPromiseStrong` handles drop while the JSC VM is still live; does not + /// run JS. The resolver's pending-native-cache slot (if any) is popped so + /// nothing is left pointing at the freed request. + /// + /// # Safety + /// `this` must be the live heap `GetAddrInfoRequest` whose `run` already + /// completed; consumed (freed) here. + #[cfg(not(windows))] + pub unsafe fn release_for_shutdown(this: *mut Self) { + // SAFETY: per fn contract — `this` is the live heap request. + unsafe { + if let Some(resolver) = (*this).resolver_for_caching { + if (*this).cache.pending_cache() { + let _ = (*resolver).get_key_host( + (*this).cache.pos_in_pending(), + PendingCacheField::PendingHostCacheNative, + ); + } + } + let mut next = (*this).head.next.take(); + while let Some(n) = next { + next = (*n.as_ptr()).next.take(); + DNSLookup::destroy(n.as_ptr()); + } + drop(bun_core::heap::take(this)); + } + } + #[cfg(windows)] pub fn on_libuv_complete(uv_info: *mut libuv::uv_getaddrinfo_t) { unsafe { @@ -1731,7 +1767,12 @@ impl CAresReverse { unsafe { let mut promise = core::mem::take(&mut (*this).promise); let global_this = (*this).global_this(); - let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + // An empty `result` means the caller's JS-result conversion threw + // (JSC termination during worker teardown). Resolving with it would + // trip `ASSERT(!target.isEmpty())` in `JSC__JSPromise__resolve`. + if !result.is_empty() { + let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + } if let Some(resolver) = (*this).resolver.as_ref() { // IntrusiveRc holds a live ref; request_completed mutates pending_requests counter only. (*resolver.as_ptr()).request_completed(); @@ -1881,7 +1922,12 @@ impl CAresLookup { unsafe { let mut promise = core::mem::take(&mut (*this).promise); let global_this = (*this).global_this(); - let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + // An empty `result` means the caller's JS-result conversion threw + // (JSC termination during worker teardown). Resolving with it would + // trip `ASSERT(!target.isEmpty())` in `JSC__JSPromise__resolve`. + if !result.is_empty() { + let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + } if let Some(resolver) = (*this).resolver.as_ref() { // IntrusiveRc holds a live ref; request_completed mutates pending_requests counter only. (*resolver.as_ptr()).request_completed(); @@ -2059,7 +2105,12 @@ impl DNSLookup { unsafe { let mut promise = core::mem::take(&mut (*this).promise); let global_this = (*this).global_this(); - let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + // An empty `result` means the caller's JS-result conversion threw + // (JSC termination during worker teardown). Resolving with it would + // trip `ASSERT(!target.isEmpty())` in `JSC__JSPromise__resolve`. + if !result.is_empty() { + let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + } if let Some(resolver) = (*this).resolver.as_ref() { // IntrusiveRc holds a live ref; request_completed mutates pending_requests counter only. (*resolver.as_ptr()).request_completed(); @@ -4526,8 +4577,9 @@ impl Resolver { pending = (*value.as_ptr()).next; if !core::ptr::eq(prev_global, new_global) { array = super::options_jsc::result_any_to_js(result, new_global) - .unwrap_or(None) - .unwrap(); // TODO: properly propagate exception upwards + .ok() + .flatten() + .unwrap_or(JSValue::ZERO); // TODO: properly propagate exception upwards prev_global = new_global; } diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 9d476d1f3d53..8957f20fc7de 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isDebug } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug, isWindows } from "harness"; import { join } from "path"; // Worker VM startup/teardown is much slower under debug and/or ASAN; these @@ -176,3 +176,66 @@ test.skipIf(!isASAN)( }, timeout, ); + +// Regression: a dns.lookup() completion dispatched on the worker thread while +// the parent's terminate() has already set the JSC termination flag would see +// the result-to-JSArray conversion throw, leaving the value as JSValue::ZERO, +// and then pass that empty value straight to JSC__JSPromise__resolve, hitting +// ASSERT(!target.isEmpty()) in debug and a near-null (0x5) SIGSEGV in release. +// +// The worker queues 128 distinct IP-literal lookups with backend:"getaddrinfo" +// so each becomes its own GetAddrInfoRequest on the work pool (the +// pending-cache hive is 32 slots, so most bypass it), blocks in Atomics.wait +// until those completions have piled up in concurrent_tasks, then the parent +// wakes it and terminates it while the drain is running. Hermetic: IP literals +// only. Windows uses uv_getaddrinfo which does not dispatch via +// GetAddrInfoRequestTask, so the reproduction path does not apply there. +test.skipIf(isWindows)( + "terminate() while dns.lookup() completions are draining does not resolve with an empty value", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const src = + 'const { parentPort, workerData } = require("node:worker_threads");' + + 'const ia = new Int32Array(workerData);' + + 'setImmediate(() => {' + + ' for (let i = 1; i <= 128; i++)' + + ' Bun.dns.lookup("127.0.0." + i, { backend: "getaddrinfo" }).catch(() => {});' + + ' parentPort.postMessage("up");' + + ' Atomics.wait(ia, 0, 0);' + + '});' + + 'setTimeout(() => {}, 100000);'; + async function once() { + const sab = new SharedArrayBuffer(4); + const ia = new Int32Array(sab); + const w = new Worker(src, { eval: true, workerData: sab }); + await new Promise((res) => w.once("message", res)); + await Bun.sleep(40); + Atomics.store(ia, 0, 1); + Atomics.notify(ia, 0); + await w.terminate(); + } + (async () => { + for (let r = 0; r < ${slow ? 6 : 10}; r++) { + await Promise.all([once(), once(), once(), once()]); + } + console.log("ok"); + })(); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("ok\n"); + expect(exitCode).toBe(0); + }, + timeout, +); From 590abbfb3823095ddee213885305b1f708c5cdc5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:24:42 +0000 Subject: [PATCH 2/3] drop pre-existing TODO comments from moved lines --- src/runtime/dns_jsc/dns.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/runtime/dns_jsc/dns.rs b/src/runtime/dns_jsc/dns.rs index de67521eed45..352954b98619 100644 --- a/src/runtime/dns_jsc/dns.rs +++ b/src/runtime/dns_jsc/dns.rs @@ -946,7 +946,7 @@ impl CAresNameInfo { // (JSC termination during worker teardown). Resolving with it would // trip `ASSERT(!target.isEmpty())` in `JSC__JSPromise__resolve`. if !result.is_empty() { - let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + let _ = promise.resolve_task(global_this, result); } // SAFETY: see fn contract. unsafe { Self::destroy(this) }; @@ -1771,7 +1771,7 @@ impl CAresReverse { // (JSC termination during worker teardown). Resolving with it would // trip `ASSERT(!target.isEmpty())` in `JSC__JSPromise__resolve`. if !result.is_empty() { - let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + let _ = promise.resolve_task(global_this, result); } if let Some(resolver) = (*this).resolver.as_ref() { // IntrusiveRc holds a live ref; request_completed mutates pending_requests counter only. @@ -1926,7 +1926,7 @@ impl CAresLookup { // (JSC termination during worker teardown). Resolving with it would // trip `ASSERT(!target.isEmpty())` in `JSC__JSPromise__resolve`. if !result.is_empty() { - let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + let _ = promise.resolve_task(global_this, result); } if let Some(resolver) = (*this).resolver.as_ref() { // IntrusiveRc holds a live ref; request_completed mutates pending_requests counter only. @@ -2109,7 +2109,7 @@ impl DNSLookup { // (JSC termination during worker teardown). Resolving with it would // trip `ASSERT(!target.isEmpty())` in `JSC__JSPromise__resolve`. if !result.is_empty() { - let _ = promise.resolve_task(global_this, result); // TODO: properly propagate exception upwards + let _ = promise.resolve_task(global_this, result); } if let Some(resolver) = (*this).resolver.as_ref() { // IntrusiveRc holds a live ref; request_completed mutates pending_requests counter only. @@ -4579,7 +4579,7 @@ impl Resolver { array = super::options_jsc::result_any_to_js(result, new_global) .ok() .flatten() - .unwrap_or(JSValue::ZERO); // TODO: properly propagate exception upwards + .unwrap_or(JSValue::ZERO); prev_global = new_global; } From ac2b7bc57c2b77a5e88fa2394cdc3d952004efeb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:39:55 +0000 Subject: [PATCH 3/3] ci: retrigger