Repository navigation
Conversation
The callback of child.send(), process.send() and Bun.spawn().send() was queued with process.nextTick when the write completed. process.nextTick captures the async context of that moment. A send that the channel did not take inside the call lost its AsyncLocalStorage store, or got the store of a later send() that completed it. A message that is still queued when send() returns now keeps its callback with the async context of the call, and the tick of a kept callback is queued in that context. A message that the channel takes inside send() queues its tick there, as before. node:child_process gives the native layer the callback of its caller, and no function when there is none. A send with no callback queues no tick.
|
Status: ready for review. How to reproduce. Save the script as const { fork } = require("node:child_process");
const { AsyncLocalStorage } = require("node:async_hooks");
const als = new AsyncLocalStorage();
const show = () => als.getStore() ?? "(no store)";
if (process.argv[2] === "child") {
process.on("message", m => {
if (m === "go") als.run("child send", () => process.send({ big: "x".repeat(1 << 20) }, () => process.send({ childCallbackStore: show() }, () => process.disconnect())));
});
} else {
const out = {};
const child = als.run("fork", () => fork(__filename, ["child"]));
child.on("message", m => { if (m.childCallbackStore) out["process.send(msg, cb) in the child"] = m.childCallbackStore; });
child.on("exit", () => { console.log(JSON.stringify(out)); process.exit(Object.values(out).every(v => v.endsWith("send")) ? 0 : 1); });
als.run("parent send", () => child.send({ big: "x".repeat(1 << 20) }, () => { out["child.send(msg, cb) in the parent"] = show(); child.send("go"); }));
}
The two fixtures that this PR adds to |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 1 minute. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Updated 3:05 PM PT - Oct 21st, 2026
❌ @robobun, your commit 6ddeff3 has some failures in 🧪 To try this PR locally: bunx bun-pr 46370That installs a local version of the PR into your bun-46370 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline nit, I also checked the callback re-attach after continue_send in serialize_and_send (src/runtime/ipc.rs): on POSIX the synchronous write completion only queues nextTicks and close_socket leaves the queue in place, so the callback lands on its own item or is ticked from the call, not on another message's item. I also checked the async-context swap in Bun__Process__queueNextTick1InStoredContext (src/jsc/bindings/BunProcess.cpp): the previous context is restored unconditionally after queueNextTick, with the only early return happening before the swap.
Extended reasoning...
The change touches the IPC send queue in src/runtime/ipc.rs and ipc_host.rs, a new C++ nextTick entry in BunProcess.cpp that swaps the global async-context internal field, and ChildProcess#send in src/js/node/child_process.ts, with new tests for callback async-context across process.send, child.send and Bun.spawn().send. It touches no auth, crypto or injection surface. The one posted finding is a test-fixture cleanup nit; the runtime change is subtle enough (async-context swapping, callback ownership across a synchronous write, ModuleGraph drop semantics) that a human look is still needed.
|
|
||
| if (process.argv[3] === "child") { | ||
| const pause = new Int32Array(new SharedArrayBuffer(4)); | ||
| while (!fs.existsSync(process.argv[2])) Atomics.wait(pause, 0, 0, 1); |
There was a problem hiding this comment.
🟡 nit (optional): when this test fails or times out, the forked grandchild keeps polling for the gate file forever and is left behind on the CI runner. The child spins in while (!fs.existsSync(process.argv[2])) Atomics.wait(pause, 0, 0, 1) with no deadline, and the gate lives in a tempDir that is removed when the test ends, so it can never appear once the parent is gone. Fix: give every gate poll loop a bounded deadline that throws, like waitFor in async-context-child_process-send.js:88-93, which covers the 2 sites. Same pattern at 2 sites (child-process-send-cb-handle-fixture.js:9, module-graph-callbacks.test.ts:315).
Why this was flagged
The handle test in test/js/node/child_process/child_process_send_cb.test.js:76-99 spawns the fixture, which fork()s itself as a child that blocks synchronously in the poll loop at test/js/node/child_process/fixtures/child-process-send-cb-handle-fixture.js:9 until the gate path exists. The gate is only written by the parent fixture inside setImmediate (line 24-26). If the parent fixture fails or the test times out, await using proc kills only the parent, using dir = tempDir(...) removes the directory, and the grandchild never processes the IPC disconnect because it never returns to the event loop; it polls at 1 ms intervals indefinitely. The same unbounded loop is in the held-child.mjs source at test/js/bun/module-graph/module-graph-callbacks.test.ts:315, though there the child is spawned directly and disposed by the test. The sibling fixture test/js/node/async_hooks/async-context/async-context-child_process-send.js:88-93 already bounds the same wait with a 30 s deadline. On the base branch none of these fixtures exist.
Verification: nit. Trigger: the parent fixture dies before its setImmediate runs. In test/js/node/child_process/fixtures/child-process-send-cb-handle-fixture.js line 9 while (!fs.existsSync(process.argv[2])) Atomics.wait(pause, 0, 0, 1); has no deadline. The gate lives in using dir = tempDir(...) and await using proc only kills the parent fixture pid, so the grandchild polls existsSync forever.
Problem
send()callback then runs with no AsyncLocalStorage store, or in the store of a latersend(). Node 26.3 uses the store of the call. Regression from more child-process #18688 (1.2.13).CallbackList::call_next_tick(src/runtime/ipc.rs:757) queued the callback withprocess.nextTickat write completion.process.nextTickcaptures the context of that moment.Bun.ModuleGraph(a second program in the process) ran as the host.Fix
SendQueue::serialize_and_sendkeeps the callback of a message still queued aftersend(), with the async context of the call.Bun__Process__queueNextTick1InStoredContextqueues its tick in that context, or in none.send()queues its tick there, as before.spawn.ipc.test.ts,child_process_send_cb.test.js,module-graph-callbacks.test.ts. Self-reviewed: 30 concerns raised, 27 addressed.Background
SendQueueholds the messages the channel did not take yet, with their callbacks.send(): every send pays. Consideredprocess.nextTickaccepting the wrapper: a send with no store gets a later store.Downsides
.text: +1,536 bytes.fork().dispose(), a waiting callback is dropped and one already queued runs. On Windows every send waits.Notes
Origin. Found by comparison with Node v26.3.0. There is no issue and no user report, so this is normal priority. #46141 fixes the same class for
node:netwrites and lists the IPC callback as not covered.Who is affected. Code that passes a callback to
child.send(), clusterworker.send(),process.send()orBun.spawn().send()and reads AsyncLocalStorage in it. The message must wait for the channel:An awaited or promisified
send()was never affected, and neither was the error argument.Rows against Node v26.3.0 (Linux x64).
bigis 1 MiB.queuedis a small message sent right after it.send()in store B completesfork()outside any storefork()inside a storefork()'uncaughtException'for a callback that throwsThe rows are the same for
child.send,process.sendandBun.spawn().send, and forjsonandadvancedserialization.What changes, by file.
ipc.rs:serialize_and_sendreads the queue after the serializer ran (a getter can send). With an empty queue and no handle it queues the message with no callback and writes it. If the message is still queued, it keeps the callback with the context of the call (CallbackContext::OfSend). If not, it queues the tick in the call, as main does. A message behind other messages, or with a handle, keeps its callback at once.CallbackContext::BehindHandleis the rule for a send held behind a handle: theBun.ModuleGraphof the sender, and no store.BunProcess.cpp,JSValue.rs:Bun__Process__queueNextTick1InStoredContextmakes the kept context current for theprocess.nextTickcall and puts the previous one back. A bare function means no context, so a latersend()that completes the message does not give it its store. This entry has one caller, the IPC send queue. It is not a general rule for kept callbacks.child_process.ts,ipc_host.rs:ChildProcess#sendgives the native layer the caller's callback. It wrapped every send in a function before, also a send with no callback. A send with no callback now queues no tick (Node: none) and keeps nothing. A send that fails is reported to a fifth argument that onlynode:child_processpasses.Measurements (linux-x64, release builds with ThinLTO of the merge base and of this branch).
send(), with a callback, with or without a store, per send: 0 capture calls, 0AsyncContextFramecells and 1 tick on both builds. Protect and unprotect calls: 1 + 1 on main, 0 + 0 here (gdb breakpoint hit counts, slope over 1,000 to 2,000 sends).child.send(msg)with no callback, per send: 1 tick and 1 + 1 protect calls on main, 0 and 0 + 0 here.perf_event_open, JIT off, minimum of 7 runs, slope over 2,000 to 6,000 sends):child.send(msg, cb)11,772 to 10,809, under a store 12,153 to 11,118.child.send(msg)11,399 to 5,436.Bun.spawn().send(msg, cb)19,818 to 19,311, under a store 19,708 to 19,521.send()and 907 wait, on both builds. Here the 907 make 907 capture calls and 907 cells (0 cells with no store). On main 907 callbacks run in no store.send()calls of libc (anLD_PRELOADcounter): in that loop 93 accepted and 907 refused on both builds. Ten runs end at 1,003 to 1,007 calls on both.SendHandle72 B andCallbackList16 B, as on main..text81,932,069 B to 81,933,605 B.Bun__Process__queueNextTick1InStoredContextis 692 B. No host function and no codegen entry is added.Bun.ModuleGraph. On main a waiting callback ran as the host:
Bun.ModuleGraph.currentwas undefined in it, a throw skipped the graph'sonError, and it ran afterdispose(). Here it runs in the graph that calledsend(), also on a channel that another graph or the host owns. Afterdispose()it is dropped, throughshouldDropCallbackOfStoppedModuleGraph, as other kept callbacks are. A tick that was queued insidesend()beforedispose()still runs, which is the rule for ticks a graph queued. So which callbacks a disposed graph still gets depends on what the channel took. This needs a yes from the owner ofBun.ModuleGraph.Still different from Node on this channel, and not changed.
fork()called inside a store: Node runs the callback in the store offork(). Bun runs it in no store, as on main. The stores of the channel would have to go under theBun.ModuleGraphframe of the sender, and no helper does that today.send(msg, handle, cb)calls back when the peer acknowledged the handle. Node calls back when the message is written.child_process_send_cb.test.jsnow pins what Bun does. On Windows the sender's handle must stay open until the peer imported it, so this needs a change of its own.Not part of this change. Other callbacks that complete later:
node:net,node:tlsandnode:httpwrites (#46141),child.stdinwrites,node:http2stream writes andsession.settings(cb). The callbacks of cluster's internal messages are not user callbacks, and Node also runs them from its message listener. The tick that closes a sentnet.Socketis queued as before.Open PRs that touch the same lines. #43081 adds
has_unacked_handle()with the predicate thatstart_messagehas inline here. The one that lands second can call it. #43000 and #45662 editSendQueuenext to this change. #38662 types the callback ofSubprocess#send, and the@ts-expect-errorinspawn.ipc.test.tsgoes when it lands.Tests.
async-context-child_process-send.jsandasync-context-child_process-send-handle.jsrun under Node and Bun throughAsyncLocalStorage-tracking.test.ts. On main they print 16 and 8 failed rows. The handle fixture does nothing on Windows, where a handle send can lose its ACK (cluster: fix two ways a Windows worker stops reacting (shared accept() race, early handle ack); fixes test-cluster-shared-leak timeouts #37815).spawn.ipc.test.ts: both sides in both serialization modes, and a getter that sends while the outersend()serializes.child_process_send_cb.test.js: ten sends that complete insidesend()create noAsyncContextFrameand run before a tick queued after them. The order of callbacks in the existing snapshot is unchanged.module-graph-callbacks.test.ts: a waiting callback runs in its graph, two graphs and the host on one channel, a held send,dispose(), and a throw that reachesonError.test-child-process-*andtest-cluster-*tests and 9 sequential ones,test/js/node/child_process/,cluster.test.ts, thespawn.ipc*tests,test/js/node/async_hooks/,module-graph-io.test.ts, andrust:check-allforx86_64-pc-windows-msvcandaarch64-apple-darwin.Self-review, not changed. Three of the 30 concerns are listed above and not fixed here: the store of
fork()for a held send, the time of the callback of a handle send, and the tick that a disposed graph queued beforedispose().