From aad58f6045b1a2b6ede3223947cb6f5362c823e1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 22:15:54 +0000 Subject: [PATCH 1/6] worker_threads: close a transferred FileHandle that the worker never received A FileHandle in transferList travels to the worker as a bare fd inside workerData. The parent handle is detached at once, and only the worker turns the marker back into a FileHandle. A worker that exits before it unpacks its workerData left the fd open for the life of the process. Each marker now carries a one-slot SharedArrayBuffer. The thread that flips the slot first owns the fd: the worker when it deserializes the marker, or the parent when it rolls the transfer back or when the worker has exited. On 'close' the parent closes every fd that nobody claimed. An object in workerData that only imitates the marker key has no claim flag and is now delivered as plain data. --- src/js/node/worker_threads.ts | 86 +++++++--- .../worker_threads/worker_threads.test.ts | 153 ++++++++++++++++++ 2 files changed, 220 insertions(+), 19 deletions(-) diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index 1658bd7cb4eb..edc25ab10f19 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -2,6 +2,7 @@ declare const self: typeof globalThis; type WebWorker = InstanceType; const EventEmitter = require("node:events"); +const { isInt32Array } = require("node:util/types"); const { SafeMap } = require("internal/primordials"); const { throwNotImplemented, warnNotImplementedOnce } = require("internal/shared"); const { @@ -356,16 +357,53 @@ function setupWorkerStdio(stdio) { // A plain string key on purpose: Symbols don't survive structured clone, and // Bun has no native HostObject hook, so the marker must ride along inside the // cloned graph (including Map/Set entries). This is in-band signaling: a user -// object that fabricates the key in workerData will deserialize on the worker -// side where node would deliver it unchanged. That's accepted - it is not a -// privilege boundary (worker threads share the parent's fd table anyway). +// object that fabricates the key and the claim flag in workerData will +// deserialize on the worker side where node would deliver it unchanged. That's +// accepted - it is not a privilege boundary (worker threads share the parent's +// fd table anyway). const kJSTransferableMarker = "__bunNodeWorkerJSTransferable"; function isJSTransferableMarker(value: object): boolean { - return ( - typeof (value as Record)[kJSTransferableMarker] === "string" && - Object.prototype.hasOwnProperty.$call(value, kJSTransferableMarker) - ); + if ( + typeof (value as Record)[kJSTransferableMarker] !== "string" || + !Object.prototype.hasOwnProperty.$call(value, kJSTransferableMarker) + ) { + return false; + } + // Exactly what claimJSTransferable() accepts, so user data that looks like a + // marker stays plain data and never throws in the worker bootstrap. + const claim = (value as Record).claim; + return isInt32Array(claim) && claim.length > 0; +} + +// Between kTransfer() on the parent and kDeserialize() on the worker the +// resource (a bare fd) has no owner. Each marker carries a one-slot +// SharedArrayBuffer, and the resource belongs to the thread that flips the slot +// first: the worker when it deserializes the marker, or the parent when it +// rolls the transfer back or finds the marker undelivered after the worker +// exited. Node closes the fd of a message that is destroyed undelivered: +// https://github.com/nodejs/node/blob/v26.3.0/src/node_file.cc#L318-L327 +function claimJSTransferable(claim: Int32Array): boolean { + return Atomics.compareExchange(claim, 0, 0, 1) === 0; +} + +function closeIfUnclaimed(data: unknown, claim: Int32Array) { + const fd = (data as any)?.fd; + if (typeof fd !== "number" || fd < 0 || !claimJSTransferable(claim)) return; + try { + require("node:fs").closeSync(fd); + } catch { + // already closed + } +} + +// Module scope on purpose. The Worker keeps this closure until it exits, and a +// closure made inside packJSTransferables() would keep that call's whole scope +// alive with it, the user's workerData graph included. +function makeReclaimJSTransferables(pending: Array<[data: unknown, claim: Int32Array]>) { + return function reclaimJSTransferables() { + for (const { 0: data, 1: claim } of pending) closeIfUnclaimed(data, claim); + }; } function deserializeJSTransferable(marker: Record): unknown { @@ -374,7 +412,7 @@ function deserializeJSTransferable(marker: Record): unknown { case "internal/fs/promises:FileHandle": { const { FileHandle, kDeserialize } = require("node:fs").promises.$data; const handle = new FileHandle(-1); - handle[kDeserialize](marker.data); + if (claimJSTransferable(marker.claim)) handle[kDeserialize](marker.data); return handle; } default: @@ -434,6 +472,7 @@ function unpackJSTransferables(value: unknown, memo?: Map): unk const kRestoreJSTransferables = Symbol("kRestoreJSTransferables"); const kFinalizeJSTransferables = Symbol("kFinalizeJSTransferables"); +const kReclaimJSTransferables = Symbol("kReclaimJSTransferables"); function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { const transferList = options?.transferList; @@ -460,9 +499,10 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { // kTransfer() neuters the handle (extracts the bare fd); if anything later // in the pack/construct sequence throws, restore the already-neutered // handles so their fds aren't orphaned. - const neutered: Array<[item: any, data: unknown]> = []; + const neutered: Array<[item: any, data: unknown, claim: Int32Array]> = []; function restoreNeutered() { - for (const { 0: item, 1: data } of neutered) { + for (const { 0: item, 1: data, 2: claim } of neutered) { + if (!claimJSTransferable(claim)) continue; try { item[kDeserialize](data); } catch { @@ -483,12 +523,15 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { ); } const extraTransfers = item[kTransferList]?.(); + // Allocated before kTransfer() so nothing can throw between neutering and the push. + const claim = new Int32Array(new SharedArrayBuffer(4)); // May throw DataCloneError (e.g. FileHandle in use); propagate synchronously like Node. const { data, deserializeInfo } = item[kTransfer](); - neutered.push([item, data]); + neutered.push([item, data, claim]); (replacements ??= new Map()).set(item, { [kJSTransferableMarker]: deserializeInfo, data, + claim, }); if ($isArray(extraTransfers)) nativeTransferList.push(...extraTransfers); } else { @@ -570,16 +613,16 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { // rollback above must still find the fd open to restore the handle (node // leaves the handle fully usable in that case). packed[kFinalizeJSTransferables] = function finalizeJSTransferables() { - for (const { 0: item, 1: data } of neutered) { - if (!usedMarkers.has(item) && typeof (data as any)?.fd === "number" && (data as any).fd >= 0) { - try { - require("node:fs").closeSync((data as any).fd); - } catch { - // already closed - } - } + for (const { 0: item, 1: data, 2: claim } of neutered) { + if (!usedMarkers.has(item)) closeIfUnclaimed(data, claim); } }; + // A worker that exits before it unpacks its workerData (the entry did not + // resolve, or terminate() got there first) leaves every marker undelivered. + // Runs on 'close', which the worker thread posts after its VM is destroyed. + const pending: Array<[data: unknown, claim: Int32Array]> = []; + for (const { 1: data, 2: claim } of neutered) pending.push([data, claim]); + packed[kReclaimJSTransferables] = makeReclaimJSTransferables(pending); return packed; } @@ -820,6 +863,7 @@ class Worker extends EventEmitter { #urlToRevoke = ""; // threadId captured for cleaning up the messaging control port on close. #messagingThreadId: number | undefined = undefined; + #reclaimJSTransferables: (() => void) | undefined = undefined; constructor(filename: string, options: NodeWorkerOptions = {}) { super(); @@ -952,6 +996,7 @@ class Worker extends EventEmitter { // The transfer is committed - release fds that were transferred but are // not referenced from workerData (nothing will deserialize them). options[kFinalizeJSTransferables]?.(); + this.#reclaimJSTransferables = options[kReclaimJSTransferables]; // Tracing active (CLI flag or dynamic enable): record the Node-style // `[worker N] ` thread-name metadata event. No-op when tracing is // off — the agent module is a tiny one-time load. @@ -1170,6 +1215,9 @@ class Worker extends EventEmitter { messaging.destroyMainThreadPort(this.#messagingThreadId); this.#messagingThreadId = undefined; } + // Close the transferred FileHandles that the worker never received. + this.#reclaimJSTransferables?.(); + this.#reclaimJSTransferables = undefined; // End captured stdio readables when the worker exits, even if it was // terminated before its own streams finished. if (this.#stdout) { diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 809f6376c75c..4375063ec42f 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -1037,6 +1037,159 @@ test("FileHandles nested in Map and Set workerData are transferred", async () => expect(message).toEqual({ sameInstance: true, text: "hello" }); }); +// The fd of a transferred FileHandle has no owner until the worker unpacks its workerData. The +// parent closes the fd of a handle that the worker never received, as node does (~TransferData), +// and leaves alone the fd of a handle that the worker did receive. +// These tests watch descriptor numbers, so they stay serial. +describe("the fd of a FileHandle transferred through workerData", () => { + // False once the descriptor is closed, or once its number belongs to another file. + function refersTo(fd: number, file: fs.Stats) { + try { + const now = fs.fstatSync(fd); + return now.ino === file.ino && now.dev === file.dev; + } catch (e: any) { + if (e.code !== "EBADF") throw e; + return false; + } + } + + // A FileHandle to transfer. Disposal closes what a failed expectation leaves open: the handle + // if it still owns the fd, or the bare fd. + async function openToTransfer(path: string) { + const fh = await fs.promises.open(path, "r"); + const fd = fh.fd; + const file = fs.fstatSync(fd); + return { + fh, + fd, + isOpen: () => refersTo(fd, file), + async [Symbol.asyncDispose]() { + if (fh.fd !== -1) await fh.close(); + else if (refersTo(fd, file)) fs.closeSync(fd); + }, + }; + } + + // open() returns the lowest free descriptor, so these take the number `fd` again once it is + // closed. A second close of that number then closes one of them. + function reopenUpTo(fd: number, path: string) { + const held = [fs.openSync(path, "r")]; + const file = fs.fstatSync(held[0]); + while (held.at(-1)! < fd) held.push(fs.openSync(path, "r")); + return { + closedByOthers: () => held.filter(descriptor => !refersTo(descriptor, file)), + [Symbol.dispose]() { + for (const descriptor of held) if (refersTo(descriptor, file)) fs.closeSync(descriptor); + }, + }; + } + + test("is closed when the worker entry does not resolve", async () => { + using dir = tempDir("worker-fh-undelivered", { "x.txt": "hello" }); + await using transferred = await openToTransfer(join(String(dir), "x.txt")); + const { fh } = transferred; + const worker = new Worker(join(String(dir), "missing.js"), { workerData: { fh }, transferList: [fh as any] }); + const errors: string[] = []; + worker.on("error", error => errors.push(error.code)); + const code = await new Promise(resolve => worker.on("exit", resolve)); + expect({ errors, code, parentFd: fh.fd, open: transferred.isOpen() }).toEqual({ + errors: ["MODULE_NOT_FOUND"], + code: 1, + parentFd: -1, + open: false, + }); + }); + + // terminate() in the same tick as the constructor stops the thread before it unpacks its + // workerData. A thread that wins that race receives the handle and owns the fd, so that + // attempt proves nothing and the next one runs. + test("is closed when terminate() stops the worker before it starts", async () => { + using dir = tempDir("worker-fh-undelivered", { "x.txt": "hello" }); + let closed = false; + for (let attempt = 0; attempt < 10 && !closed; attempt++) { + // The worker is gone at disposal, so nothing else can close the fd of an attempt it won. + await using transferred = await openToTransfer(join(String(dir), "x.txt")); + const { fh } = transferred; + const worker = new Worker("setInterval(() => {}, 1000)", { + eval: true, + workerData: { fh }, + transferList: [fh as any], + }); + await worker.terminate(); + expect(fh.fd).toBe(-1); + closed = !transferred.isOpen(); + } + expect(closed).toBe(true); + }); + + // A handle that workerData does not reference is closed by the constructor. The exit of a + // worker that never started must not close that number a second time. + test("is closed only once when workerData does not reference the handle", async () => { + using dir = tempDir("worker-fh-unreferenced", { "x.txt": "hello", "y.txt": "world" }); + const other = join(String(dir), "y.txt"); + // A starting worker thread opens descriptors of its own. It takes these lower numbers, which + // leaves the number of the handle for reopenUpTo(). + const parked = Array.from({ length: 16 }, () => fs.openSync(other, "r")); + await using transferred = await openToTransfer(join(String(dir), "x.txt")); + for (const descriptor of parked) fs.closeSync(descriptor); + const { fh } = transferred; + const worker = new Worker(join(String(dir), "missing.js"), { workerData: {}, transferList: [fh as any] }); + const closedByConstructor = !transferred.isOpen(); + using reopened = reopenUpTo(transferred.fd, other); + const errors: string[] = []; + worker.on("error", error => errors.push(error.code)); + const code = await new Promise(resolve => worker.on("exit", resolve)); + expect({ errors, code, closedByConstructor, closedAgain: reopened.closedByOthers() }).toEqual({ + errors: ["MODULE_NOT_FOUND"], + code: 1, + closedByConstructor: true, + closedAgain: [], + }); + }); + + // Fails when the claim of the worker does not reach the parent: the parent then closes the + // number of a handle that the worker received and closed, which by then is another file's. + test("is not closed by the parent when the worker received the handle", async () => { + using dir = tempDir("worker-fh-delivered", { "x.txt": "hello", "y.txt": "world" }); + await using transferred = await openToTransfer(join(String(dir), "x.txt")); + const { fh } = transferred; + await using worker = new Worker( + `const { workerData, parentPort } = require("node:worker_threads"); + parentPort.once("message", () => {}); + workerData.fh.close().then(() => parentPort.postMessage("closed"));`, + { eval: true, workerData: { fh }, transferList: [fh as any] }, + ); + const [message] = await once(worker, "message"); + using reopened = reopenUpTo(transferred.fd, join(String(dir), "y.txt")); + worker.postMessage("exit"); + const [code] = await once(worker, "exit"); + expect({ message, code, closedByParent: reopened.closedByOthers() }).toEqual({ + message: "closed", + code: 0, + closedByParent: [], + }); + }); + + test.each([ + ["no claim flag", undefined], + ["a claim flag of the wrong type", new Float64Array(1)], + ["an empty claim flag", new Int32Array(0)], + ])("workerData that imitates a marker with %s stays plain data", async (_, claim) => { + const imitation = { + __bunNodeWorkerJSTransferable: "internal/fs/promises:FileHandle", + data: { fd: 1 << 20 }, + claim, + }; + await using worker = new Worker( + `const { workerData, parentPort } = require("node:worker_threads"); + parentPort.postMessage(workerData);`, + { eval: true, workerData: { imitation } }, + ); + const [message] = await once(worker, "message"); + expect(message).toEqual({ imitation }); + }); +}); + test("MessagePort.hasRef() reports actual loop-ref state", () => { const { port1 } = new MessageChannel(); expect(port1.hasRef()).toBe(false); From 97d98aa26797da4bafaecc646e229d4315f7ee21 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 21:45:13 +0000 Subject: [PATCH 2/6] worker_threads: make and flip the transfer claim flag natively The claim flag no longer needs the SharedArrayBuffer, Int32Array and Atomics globals, which user code can replace or remove. Two native helpers make the 4 shared bytes and flip them with a compare-and-swap. A marker is now also checked for its data: an object in workerData that has the marker key but no numeric data.fd, or no valid claim flag, stays plain data and cannot throw in the worker bootstrap. The worker claims the fd after it loaded node:fs and made the handle, so that a terminate() cannot land in that load with the fd already claimed. --- src/js/node/worker_threads.ts | 73 ++++++++++--------- src/jsc/bindings/webcore/Worker.cpp | 36 ++++++++- .../worker_threads/worker_threads.test.ts | 50 ++++++++++--- 3 files changed, 115 insertions(+), 44 deletions(-) diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index edc25ab10f19..e7b9848368a4 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -2,7 +2,6 @@ declare const self: typeof globalThis; type WebWorker = InstanceType; const EventEmitter = require("node:events"); -const { isInt32Array } = require("node:util/types"); const { SafeMap } = require("internal/primordials"); const { throwNotImplemented, warnNotImplementedOnce } = require("internal/shared"); const { @@ -77,6 +76,8 @@ const { 14: MessageChannel, 15: BroadcastChannel, 16: WebWorker, + 17: _createJSTransferableClaim, + 18: _claimJSTransferable, } = $cpp("Worker.cpp", "createNodeWorkerThreadsBinding") as [ unknown, number, @@ -98,6 +99,9 @@ const { // instance. This is so that it can emit the `worker` event on the process with the // node:worker_threads instance instead of the Web Worker instance. new (...args: [...ConstructorParameters, nodeWorker: Worker]) => WebWorker, + () => SharedArrayBuffer, + // true: this call took the claim. false: another thread took it first. undefined: not a claim flag. + (claim: unknown) => boolean | undefined, ]; type NodeWorkerOptions = import("node:worker_threads").WorkerOptions; @@ -357,39 +361,30 @@ function setupWorkerStdio(stdio) { // A plain string key on purpose: Symbols don't survive structured clone, and // Bun has no native HostObject hook, so the marker must ride along inside the // cloned graph (including Map/Set entries). This is in-band signaling: a user -// object that fabricates the key and the claim flag in workerData will -// deserialize on the worker side where node would deliver it unchanged. That's -// accepted - it is not a privilege boundary (worker threads share the parent's -// fd table anyway). +// object that fabricates the whole marker (key, data and claim flag) in +// workerData will deserialize on the worker side where node would deliver it +// unchanged. That's accepted - it is not a privilege boundary (worker threads +// share the parent's fd table anyway). const kJSTransferableMarker = "__bunNodeWorkerJSTransferable"; function isJSTransferableMarker(value: object): boolean { - if ( - typeof (value as Record)[kJSTransferableMarker] !== "string" || - !Object.prototype.hasOwnProperty.$call(value, kJSTransferableMarker) - ) { - return false; - } - // Exactly what claimJSTransferable() accepts, so user data that looks like a - // marker stays plain data and never throws in the worker bootstrap. - const claim = (value as Record).claim; - return isInt32Array(claim) && claim.length > 0; + return ( + typeof (value as Record)[kJSTransferableMarker] === "string" && + Object.prototype.hasOwnProperty.$call(value, kJSTransferableMarker) + ); } // Between kTransfer() on the parent and kDeserialize() on the worker the -// resource (a bare fd) has no owner. Each marker carries a one-slot -// SharedArrayBuffer, and the resource belongs to the thread that flips the slot -// first: the worker when it deserializes the marker, or the parent when it -// rolls the transfer back or finds the marker undelivered after the worker -// exited. Node closes the fd of a message that is destroyed undelivered: +// resource (a bare fd) has no owner. Each marker carries a claim flag (4 shared +// bytes, made and flipped natively), and the resource belongs to the thread +// that flips it first: the worker when it deserializes the marker, or the +// parent when it rolls the transfer back or finds the marker undelivered after +// the worker exited. Node closes the fd of a message that is destroyed +// undelivered: // https://github.com/nodejs/node/blob/v26.3.0/src/node_file.cc#L318-L327 -function claimJSTransferable(claim: Int32Array): boolean { - return Atomics.compareExchange(claim, 0, 0, 1) === 0; -} - -function closeIfUnclaimed(data: unknown, claim: Int32Array) { +function closeIfUnclaimed(data: unknown, claim: SharedArrayBuffer) { const fd = (data as any)?.fd; - if (typeof fd !== "number" || fd < 0 || !claimJSTransferable(claim)) return; + if (typeof fd !== "number" || fd < 0 || _claimJSTransferable(claim) !== true) return; try { require("node:fs").closeSync(fd); } catch { @@ -400,19 +395,27 @@ function closeIfUnclaimed(data: unknown, claim: Int32Array) { // Module scope on purpose. The Worker keeps this closure until it exits, and a // closure made inside packJSTransferables() would keep that call's whole scope // alive with it, the user's workerData graph included. -function makeReclaimJSTransferables(pending: Array<[data: unknown, claim: Int32Array]>) { +function makeReclaimJSTransferables(pending: Array<[data: unknown, claim: SharedArrayBuffer]>) { return function reclaimJSTransferables() { for (const { 0: data, 1: claim } of pending) closeIfUnclaimed(data, claim); }; } +// Returns undefined for user data that only looks like a marker. It stays plain +// data, as in node, and must not throw here: this runs in the worker bootstrap. function deserializeJSTransferable(marker: Record): unknown { const deserializeInfo = marker[kJSTransferableMarker]; switch (deserializeInfo) { case "internal/fs/promises:FileHandle": { + const data = marker.data; + if (data === null || typeof data !== "object" || typeof data.fd !== "number") return undefined; const { FileHandle, kDeserialize } = require("node:fs").promises.$data; const handle = new FileHandle(-1); - if (claimJSTransferable(marker.claim)) handle[kDeserialize](marker.data); + // Claim last. A terminate() that lands between the claim and kDeserialize() + // leaks the fd, so nothing slow (the node:fs load above) may sit in between. + const claimed = _claimJSTransferable(marker.claim); + if (claimed === undefined) return undefined; + if (claimed) handle[kDeserialize](data); return handle; } default: @@ -432,8 +435,10 @@ function unpackJSTransferables(value: unknown, memo?: Map): unk if (cached !== undefined) return cached; if (isJSTransferableMarker(value)) { const instance = deserializeJSTransferable(value as Record); - memo.set(value, instance); - return instance; + if (instance !== undefined) { + memo.set(value, instance); + return instance; + } } memo.set(value, value); if ($isArray(value)) { @@ -499,10 +504,10 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { // kTransfer() neuters the handle (extracts the bare fd); if anything later // in the pack/construct sequence throws, restore the already-neutered // handles so their fds aren't orphaned. - const neutered: Array<[item: any, data: unknown, claim: Int32Array]> = []; + const neutered: Array<[item: any, data: unknown, claim: SharedArrayBuffer]> = []; function restoreNeutered() { for (const { 0: item, 1: data, 2: claim } of neutered) { - if (!claimJSTransferable(claim)) continue; + if (_claimJSTransferable(claim) !== true) continue; try { item[kDeserialize](data); } catch { @@ -524,7 +529,7 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { } const extraTransfers = item[kTransferList]?.(); // Allocated before kTransfer() so nothing can throw between neutering and the push. - const claim = new Int32Array(new SharedArrayBuffer(4)); + const claim = _createJSTransferableClaim(); // May throw DataCloneError (e.g. FileHandle in use); propagate synchronously like Node. const { data, deserializeInfo } = item[kTransfer](); neutered.push([item, data, claim]); @@ -620,7 +625,7 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { // A worker that exits before it unpacks its workerData (the entry did not // resolve, or terminate() got there first) leaves every marker undelivered. // Runs on 'close', which the worker thread posts after its VM is destroyed. - const pending: Array<[data: unknown, claim: Int32Array]> = []; + const pending: Array<[data: unknown, claim: SharedArrayBuffer]> = []; for (const { 1: data, 2: claim } of neutered) pending.push([data, claim]); packed[kReclaimJSTransferables] = makeReclaimJSTransferables(pending); return packed; diff --git a/src/jsc/bindings/webcore/Worker.cpp b/src/jsc/bindings/webcore/Worker.cpp index 15b0b9ff83e8..1d92bde21d6c 100644 --- a/src/jsc/bindings/webcore/Worker.cpp +++ b/src/jsc/bindings/webcore/Worker.cpp @@ -286,6 +286,36 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionSetEntryEvaluatedHook, (JSC::JSGlobalObject * return JSC::JSValue::encode(jsUndefined()); } +// The claim flag of a resource in transit to a worker (worker_threads.ts, packJSTransferables): one +// shared int32 that rides inside the cloned workerData. Native so that neither side depends on the +// SharedArrayBuffer and Atomics globals, which user code can replace. +JSC_DEFINE_HOST_FUNCTION(jsFunctionCreateJSTransferableClaim, (JSGlobalObject * lexicalGlobalObject, CallFrame*)) +{ + auto& vm = lexicalGlobalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + RefPtr buffer = ArrayBuffer::tryCreate(sizeof(int32_t), 1); + if (!buffer) [[unlikely]] { + throwOutOfMemoryError(lexicalGlobalObject, scope); + return {}; + } + buffer->makeShared(); + return JSValue::encode(JSArrayBuffer::create(vm, lexicalGlobalObject->arrayBufferStructure(ArrayBufferSharingMode::Shared), WTF::move(buffer))); +} + +// true: this call flipped the flag, so the caller now owns the resource. false: another caller +// flipped it first. undefined: the argument is not a claim flag. +JSC_DEFINE_HOST_FUNCTION(jsFunctionClaimJSTransferable, (JSGlobalObject*, CallFrame* callFrame)) +{ + auto* jsBuffer = dynamicDowncast(callFrame->argument(0)); + if (!jsBuffer) + return JSValue::encode(jsUndefined()); + auto* buffer = jsBuffer->impl(); + if (!buffer->isShared() || buffer->byteLength() < sizeof(int32_t)) + return JSValue::encode(jsUndefined()); + auto* flag = static_cast(buffer->data()); + return JSValue::encode(jsBoolean(WTF::atomicCompareExchangeStrong(flag, 0, 1) == 0)); +} + JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) { VM& vm = globalObject->vm(); @@ -342,7 +372,7 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) bool isNodeWorker = proxy && proxy->options().kind == WorkerOptions::Kind::Node; - JSObject* array = constructEmptyArray(globalObject, nullptr, 17); + JSObject* array = constructEmptyArray(globalObject, nullptr, 19); RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 0, workerData); RETURN_IF_EXCEPTION(scope, {}); @@ -380,6 +410,10 @@ JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) RETURN_IF_EXCEPTION(scope, {}); array->putDirectIndex(globalObject, 16, JSWorker::getConstructor(vm, globalObject)); RETURN_IF_EXCEPTION(scope, {}); + array->putDirectIndex(globalObject, 17, JSFunction::create(vm, globalObject, 0, "createJSTransferableClaim"_s, jsFunctionCreateJSTransferableClaim, ImplementationVisibility::Public, NoIntrinsic)); + RETURN_IF_EXCEPTION(scope, {}); + array->putDirectIndex(globalObject, 18, JSFunction::create(vm, globalObject, 1, "claimJSTransferable"_s, jsFunctionClaimJSTransferable, ImplementationVisibility::Public, NoIntrinsic)); + RETURN_IF_EXCEPTION(scope, {}); return array; } diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 4375063ec42f..855a23ffd4cb 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -1170,16 +1170,48 @@ describe("the fd of a FileHandle transferred through workerData", () => { }); }); + // The claim flag is made and flipped natively. Code that replaces these globals (a DOM shim, a + // hardened realm) keeps the transfer that it had before the flag existed. + test("is transferred when user code removed SharedArrayBuffer and Atomics", async () => { + using dir = tempDir("worker-fh-no-sab", { "x.txt": "hello" }); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const { Worker } = require("node:worker_threads"); + const fs = require("node:fs"); + globalThis.SharedArrayBuffer = globalThis.Atomics = globalThis.Int32Array = undefined; + fs.promises.open("x.txt", "r").then(fh => { + const worker = new Worker( + \`const { workerData, parentPort } = require("node:worker_threads"); + workerData.fh.readFile("utf8").then(text => workerData.fh.close().then(() => parentPort.postMessage(text)));\`, + { eval: true, workerData: { fh }, transferList: [fh] }, + ); + worker.on("message", text => console.log(JSON.stringify({ text, parentFd: fh.fd }))); + });`, + ], + env: bunEnv, + cwd: String(dir), + 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: JSON.stringify({ text: "hello", parentFd: -1 }), + stderr: "", + exitCode: 0, + }); + }); + + // The worker unpacks workerData before user code runs, so a lookalike must not throw there. test.each([ - ["no claim flag", undefined], - ["a claim flag of the wrong type", new Float64Array(1)], - ["an empty claim flag", new Int32Array(0)], - ])("workerData that imitates a marker with %s stays plain data", async (_, claim) => { - const imitation = { - __bunNodeWorkerJSTransferable: "internal/fs/promises:FileHandle", - data: { fd: 1 << 20 }, - claim, - }; + ["no claim flag", { data: { fd: 1 << 20 } }], + ["a claim flag of the wrong type", { data: { fd: 1 << 20 }, claim: new Int32Array(1) }], + ["a claim flag that is too small", { data: { fd: 1 << 20 }, claim: new SharedArrayBuffer(2) }], + ["a claim flag and no fd", { data: {}, claim: new SharedArrayBuffer(4) }], + ["a claim flag and no data", { claim: new SharedArrayBuffer(4) }], + ])("workerData that imitates a marker with %s stays plain data", async (_, rest) => { + const imitation = { __bunNodeWorkerJSTransferable: "internal/fs/promises:FileHandle", ...rest }; await using worker = new Worker( `const { workerData, parentPort } = require("node:worker_threads"); parentPort.postMessage(workerData);`, From bc2ac7f2bb367ee19286490ca939b1ae825a2c47 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 22:28:17 +0000 Subject: [PATCH 3/6] worker_threads: keep transfer claims in a native registry The claim of a FileHandle in transit is now a random id in a process-wide native set. The thread that removes the id owns the fd. The marker carries the id as a plain number, so the transfer no longer depends on a SharedArrayBuffer that the structured clone must accept, and an object in workerData that imitates a marker cannot hold a live id. Also shorten the comments to one line each. --- src/js/node/worker_threads.ts | 58 +++++++------------ src/jsc/bindings/webcore/Worker.cpp | 52 +++++++++-------- .../worker_threads/worker_threads.test.ts | 41 +++++-------- 3 files changed, 64 insertions(+), 87 deletions(-) diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index e7b9848368a4..1647403f54f7 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -99,9 +99,9 @@ const { // instance. This is so that it can emit the `worker` event on the process with the // node:worker_threads instance instead of the Web Worker instance. new (...args: [...ConstructorParameters, nodeWorker: Worker]) => WebWorker, - () => SharedArrayBuffer, - // true: this call took the claim. false: another thread took it first. undefined: not a claim flag. - (claim: unknown) => boolean | undefined, + () => number, + // true: the id was live, and the caller now owns the resource in transit. + (claim: unknown) => boolean, ]; type NodeWorkerOptions = import("node:worker_threads").WorkerOptions; @@ -360,11 +360,8 @@ function setupWorkerStdio(stdio) { // on receive, markers are swapped back for reconstructed instances. // A plain string key on purpose: Symbols don't survive structured clone, and // Bun has no native HostObject hook, so the marker must ride along inside the -// cloned graph (including Map/Set entries). This is in-band signaling: a user -// object that fabricates the whole marker (key, data and claim flag) in -// workerData will deserialize on the worker side where node would deliver it -// unchanged. That's accepted - it is not a privilege boundary (worker threads -// share the parent's fd table anyway). +// cloned graph (including Map/Set entries). A user object with the same key +// stays plain data, as in node: it cannot hold a live claim id. const kJSTransferableMarker = "__bunNodeWorkerJSTransferable"; function isJSTransferableMarker(value: object): boolean { @@ -374,17 +371,10 @@ function isJSTransferableMarker(value: object): boolean { ); } -// Between kTransfer() on the parent and kDeserialize() on the worker the -// resource (a bare fd) has no owner. Each marker carries a claim flag (4 shared -// bytes, made and flipped natively), and the resource belongs to the thread -// that flips it first: the worker when it deserializes the marker, or the -// parent when it rolls the transfer back or finds the marker undelivered after -// the worker exited. Node closes the fd of a message that is destroyed -// undelivered: -// https://github.com/nodejs/node/blob/v26.3.0/src/node_file.cc#L318-L327 -function closeIfUnclaimed(data: unknown, claim: SharedArrayBuffer) { +// The first thread to take a marker's claim id owns its fd, so one fd is never closed or restored twice. +function closeIfUnclaimed(data: unknown, claim: number) { const fd = (data as any)?.fd; - if (typeof fd !== "number" || fd < 0 || _claimJSTransferable(claim) !== true) return; + if (typeof fd !== "number" || fd < 0 || !_claimJSTransferable(claim)) return; try { require("node:fs").closeSync(fd); } catch { @@ -392,17 +382,14 @@ function closeIfUnclaimed(data: unknown, claim: SharedArrayBuffer) { } } -// Module scope on purpose. The Worker keeps this closure until it exits, and a -// closure made inside packJSTransferables() would keep that call's whole scope -// alive with it, the user's workerData graph included. -function makeReclaimJSTransferables(pending: Array<[data: unknown, claim: SharedArrayBuffer]>) { +// Module scope: a closure made in packJSTransferables() would pin the user's workerData graph until the worker exits. +function makeReclaimJSTransferables(pending: Array<[data: unknown, claim: number]>) { return function reclaimJSTransferables() { for (const { 0: data, 1: claim } of pending) closeIfUnclaimed(data, claim); }; } -// Returns undefined for user data that only looks like a marker. It stays plain -// data, as in node, and must not throw here: this runs in the worker bootstrap. +// undefined: user data that only looks like a marker. It stays plain data and must not throw in the worker bootstrap. function deserializeJSTransferable(marker: Record): unknown { const deserializeInfo = marker[kJSTransferableMarker]; switch (deserializeInfo) { @@ -411,11 +398,9 @@ function deserializeJSTransferable(marker: Record): unknown { if (data === null || typeof data !== "object" || typeof data.fd !== "number") return undefined; const { FileHandle, kDeserialize } = require("node:fs").promises.$data; const handle = new FileHandle(-1); - // Claim last. A terminate() that lands between the claim and kDeserialize() - // leaks the fd, so nothing slow (the node:fs load above) may sit in between. - const claimed = _claimJSTransferable(marker.claim); - if (claimed === undefined) return undefined; - if (claimed) handle[kDeserialize](data); + // Claim last: a terminate() between the claim and kDeserialize() leaks the fd, so the node:fs load stays above. + if (!_claimJSTransferable(marker.claim)) return undefined; + handle[kDeserialize](data); return handle; } default: @@ -504,10 +489,10 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { // kTransfer() neuters the handle (extracts the bare fd); if anything later // in the pack/construct sequence throws, restore the already-neutered // handles so their fds aren't orphaned. - const neutered: Array<[item: any, data: unknown, claim: SharedArrayBuffer]> = []; + const neutered: Array<[item: any, data: unknown, claim: number]> = []; function restoreNeutered() { for (const { 0: item, 1: data, 2: claim } of neutered) { - if (_claimJSTransferable(claim) !== true) continue; + if (!_claimJSTransferable(claim)) continue; try { item[kDeserialize](data); } catch { @@ -528,10 +513,9 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { ); } const extraTransfers = item[kTransferList]?.(); - // Allocated before kTransfer() so nothing can throw between neutering and the push. - const claim = _createJSTransferableClaim(); // May throw DataCloneError (e.g. FileHandle in use); propagate synchronously like Node. const { data, deserializeInfo } = item[kTransfer](); + const claim = _createJSTransferableClaim(); neutered.push([item, data, claim]); (replacements ??= new Map()).set(item, { [kJSTransferableMarker]: deserializeInfo, @@ -622,10 +606,8 @@ function packJSTransferables(options: NodeWorkerOptions): NodeWorkerOptions { if (!usedMarkers.has(item)) closeIfUnclaimed(data, claim); } }; - // A worker that exits before it unpacks its workerData (the entry did not - // resolve, or terminate() got there first) leaves every marker undelivered. - // Runs on 'close', which the worker thread posts after its VM is destroyed. - const pending: Array<[data: unknown, claim: SharedArrayBuffer]> = []; + // Runs on 'close', which the worker thread posts after its VM is destroyed, so every claim is final by then. + const pending: Array<[data: unknown, claim: number]> = []; for (const { 1: data, 2: claim } of neutered) pending.push([data, claim]); packed[kReclaimJSTransferables] = makeReclaimJSTransferables(pending); return packed; @@ -1220,7 +1202,7 @@ class Worker extends EventEmitter { messaging.destroyMainThreadPort(this.#messagingThreadId); this.#messagingThreadId = undefined; } - // Close the transferred FileHandles that the worker never received. + // node closes an undelivered fd too: https://github.com/nodejs/node/blob/v26.3.0/src/node_file.cc#L318-L327 this.#reclaimJSTransferables?.(); this.#reclaimJSTransferables = undefined; // End captured stdio readables when the worker exits, even if it was diff --git a/src/jsc/bindings/webcore/Worker.cpp b/src/jsc/bindings/webcore/Worker.cpp index 1d92bde21d6c..90e33d6f572f 100644 --- a/src/jsc/bindings/webcore/Worker.cpp +++ b/src/jsc/bindings/webcore/Worker.cpp @@ -37,6 +37,10 @@ #include #include #include +#include +#include +#include +#include #include "SerializedScriptValue.h" #include "ScriptExecutionContext.h" #include @@ -286,34 +290,36 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionSetEntryEvaluatedHook, (JSC::JSGlobalObject * return JSC::JSValue::encode(jsUndefined()); } -// The claim flag of a resource in transit to a worker (worker_threads.ts, packJSTransferables): one -// shared int32 that rides inside the cloned workerData. Native so that neither side depends on the -// SharedArrayBuffer and Atomics globals, which user code can replace. -JSC_DEFINE_HOST_FUNCTION(jsFunctionCreateJSTransferableClaim, (JSGlobalObject * lexicalGlobalObject, CallFrame*)) +// Ids of resources in transit to a worker (worker_threads.ts). Process-wide: the parent VM makes one, the worker VM takes it. +static Lock s_transferClaimsLock; +static HashSet& transferClaims() WTF_REQUIRES_LOCK(s_transferClaimsLock) { - auto& vm = lexicalGlobalObject->vm(); - auto scope = DECLARE_THROW_SCOPE(vm); - RefPtr buffer = ArrayBuffer::tryCreate(sizeof(int32_t), 1); - if (!buffer) [[unlikely]] { - throwOutOfMemoryError(lexicalGlobalObject, scope); - return {}; - } - buffer->makeShared(); - return JSValue::encode(JSArrayBuffer::create(vm, lexicalGlobalObject->arrayBufferStructure(ArrayBufferSharingMode::Shared), WTF::move(buffer))); + static NeverDestroyed> claims; + return claims.get(); } -// true: this call flipped the flag, so the caller now owns the resource. false: another caller -// flipped it first. undefined: the argument is not a claim flag. +JSC_DEFINE_HOST_FUNCTION(jsFunctionCreateJSTransferableClaim, (JSGlobalObject*, CallFrame*)) +{ + Locker locker { s_transferClaimsLock }; + uint64_t id; + do { + // 53 bits, so the id is exact as a JS number. 0 is the empty value of the set. + id = cryptographicallyRandomNumber() >> 11; + } while (!id || !transferClaims().add(id).isNewEntry); + return JSValue::encode(jsNumber(static_cast(id))); +} + +// true: the id was live and the caller now owns the resource. Anything else that user data can hold is false. JSC_DEFINE_HOST_FUNCTION(jsFunctionClaimJSTransferable, (JSGlobalObject*, CallFrame* callFrame)) { - auto* jsBuffer = dynamicDowncast(callFrame->argument(0)); - if (!jsBuffer) - return JSValue::encode(jsUndefined()); - auto* buffer = jsBuffer->impl(); - if (!buffer->isShared() || buffer->byteLength() < sizeof(int32_t)) - return JSValue::encode(jsUndefined()); - auto* flag = static_cast(buffer->data()); - return JSValue::encode(jsBoolean(WTF::atomicCompareExchangeStrong(flag, 0, 1) == 0)); + JSValue value = callFrame->argument(0); + if (!value.isNumber()) + return JSValue::encode(jsBoolean(false)); + double number = value.asNumber(); + if (!(number >= 1 && number < 9007199254740992.0) || number != std::trunc(number)) + return JSValue::encode(jsBoolean(false)); + Locker locker { s_transferClaimsLock }; + return JSValue::encode(jsBoolean(transferClaims().remove(static_cast(number)))); } JSValue createNodeWorkerThreadsBinding(Zig::GlobalObject* globalObject) diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 855a23ffd4cb..92b59e33817f 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -1037,9 +1037,6 @@ test("FileHandles nested in Map and Set workerData are transferred", async () => expect(message).toEqual({ sameInstance: true, text: "hello" }); }); -// The fd of a transferred FileHandle has no owner until the worker unpacks its workerData. The -// parent closes the fd of a handle that the worker never received, as node does (~TransferData), -// and leaves alone the fd of a handle that the worker did receive. // These tests watch descriptor numbers, so they stay serial. describe("the fd of a FileHandle transferred through workerData", () => { // False once the descriptor is closed, or once its number belongs to another file. @@ -1053,8 +1050,7 @@ describe("the fd of a FileHandle transferred through workerData", () => { } } - // A FileHandle to transfer. Disposal closes what a failed expectation leaves open: the handle - // if it still owns the fd, or the bare fd. + // Disposal closes what a failed expectation leaves open: the handle if it still owns the fd, else the bare fd. async function openToTransfer(path: string) { const fh = await fs.promises.open(path, "r"); const fd = fh.fd; @@ -1070,8 +1066,7 @@ describe("the fd of a FileHandle transferred through workerData", () => { }; } - // open() returns the lowest free descriptor, so these take the number `fd` again once it is - // closed. A second close of that number then closes one of them. + // open() returns the lowest free descriptor, so these take the closed number `fd` again. A second close hits one. function reopenUpTo(fd: number, path: string) { const held = [fs.openSync(path, "r")]; const file = fs.fstatSync(held[0]); @@ -1100,9 +1095,7 @@ describe("the fd of a FileHandle transferred through workerData", () => { }); }); - // terminate() in the same tick as the constructor stops the thread before it unpacks its - // workerData. A thread that wins that race receives the handle and owns the fd, so that - // attempt proves nothing and the next one runs. + // A thread that wins the race against terminate() receives the handle and owns the fd, so that attempt is repeated. test("is closed when terminate() stops the worker before it starts", async () => { using dir = tempDir("worker-fh-undelivered", { "x.txt": "hello" }); let closed = false; @@ -1122,13 +1115,11 @@ describe("the fd of a FileHandle transferred through workerData", () => { expect(closed).toBe(true); }); - // A handle that workerData does not reference is closed by the constructor. The exit of a - // worker that never started must not close that number a second time. + // The constructor closes a handle that workerData does not reference. The worker's exit must not close it again. test("is closed only once when workerData does not reference the handle", async () => { using dir = tempDir("worker-fh-unreferenced", { "x.txt": "hello", "y.txt": "world" }); const other = join(String(dir), "y.txt"); - // A starting worker thread opens descriptors of its own. It takes these lower numbers, which - // leaves the number of the handle for reopenUpTo(). + // A starting worker thread opens descriptors of its own. It takes these lower numbers, not the handle's. const parked = Array.from({ length: 16 }, () => fs.openSync(other, "r")); await using transferred = await openToTransfer(join(String(dir), "x.txt")); for (const descriptor of parked) fs.closeSync(descriptor); @@ -1147,8 +1138,7 @@ describe("the fd of a FileHandle transferred through workerData", () => { }); }); - // Fails when the claim of the worker does not reach the parent: the parent then closes the - // number of a handle that the worker received and closed, which by then is another file's. + // Fails when the worker's claim does not reach the parent: the parent then closes a number that is another file's. test("is not closed by the parent when the worker received the handle", async () => { using dir = tempDir("worker-fh-delivered", { "x.txt": "hello", "y.txt": "world" }); await using transferred = await openToTransfer(join(String(dir), "x.txt")); @@ -1170,9 +1160,8 @@ describe("the fd of a FileHandle transferred through workerData", () => { }); }); - // The claim flag is made and flipped natively. Code that replaces these globals (a DOM shim, a - // hardened realm) keeps the transfer that it had before the flag existed. - test("is transferred when user code removed SharedArrayBuffer and Atomics", async () => { + // The claim is a plain number, so the transfer needs neither the SharedArrayBuffer global nor the JSC option. + test("is transferred when SharedArrayBuffer is not available", async () => { using dir = tempDir("worker-fh-no-sab", { "x.txt": "hello" }); await using proc = Bun.spawn({ cmd: [ @@ -1180,7 +1169,7 @@ describe("the fd of a FileHandle transferred through workerData", () => { "-e", `const { Worker } = require("node:worker_threads"); const fs = require("node:fs"); - globalThis.SharedArrayBuffer = globalThis.Atomics = globalThis.Int32Array = undefined; + globalThis.SharedArrayBuffer = globalThis.Atomics = undefined; fs.promises.open("x.txt", "r").then(fh => { const worker = new Worker( \`const { workerData, parentPort } = require("node:worker_threads"); @@ -1190,7 +1179,7 @@ describe("the fd of a FileHandle transferred through workerData", () => { worker.on("message", text => console.log(JSON.stringify({ text, parentFd: fh.fd }))); });`, ], - env: bunEnv, + env: { ...bunEnv, BUN_JSC_useSharedArrayBuffer: "0" }, cwd: String(dir), stdout: "pipe", stderr: "pipe", @@ -1205,11 +1194,11 @@ describe("the fd of a FileHandle transferred through workerData", () => { // The worker unpacks workerData before user code runs, so a lookalike must not throw there. test.each([ - ["no claim flag", { data: { fd: 1 << 20 } }], - ["a claim flag of the wrong type", { data: { fd: 1 << 20 }, claim: new Int32Array(1) }], - ["a claim flag that is too small", { data: { fd: 1 << 20 }, claim: new SharedArrayBuffer(2) }], - ["a claim flag and no fd", { data: {}, claim: new SharedArrayBuffer(4) }], - ["a claim flag and no data", { claim: new SharedArrayBuffer(4) }], + ["no claim id", { data: { fd: 1 << 20 } }], + ["a claim id that is not live", { data: { fd: 1 << 20 }, claim: 12345 }], + ["a claim id of the wrong type", { data: { fd: 1 << 20 }, claim: "12345" }], + ["no fd", { data: {}, claim: 12345 }], + ["no data", { claim: 12345 }], ])("workerData that imitates a marker with %s stays plain data", async (_, rest) => { const imitation = { __bunNodeWorkerJSTransferable: "internal/fs/promises:FileHandle", ...rest }; await using worker = new Worker( From 2fc5d114375430d041e3611f15fefb9eb015e175 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 22:34:52 +0000 Subject: [PATCH 4/6] worker_threads: drop a comment sentence that is no longer true An object in workerData that fabricates the marker key no longer deserializes, because it cannot hold a live claim id. --- src/js/node/worker_threads.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index 1647403f54f7..8030a84d74e6 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -360,8 +360,7 @@ function setupWorkerStdio(stdio) { // on receive, markers are swapped back for reconstructed instances. // A plain string key on purpose: Symbols don't survive structured clone, and // Bun has no native HostObject hook, so the marker must ride along inside the -// cloned graph (including Map/Set entries). A user object with the same key -// stays plain data, as in node: it cannot hold a live claim id. +// cloned graph (including Map/Set entries). const kJSTransferableMarker = "__bunNodeWorkerJSTransferable"; function isJSTransferableMarker(value: object): boolean { From 6c5bebdca77ae635b529b8eddbafefa04e8be51d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:00:00 +0000 Subject: [PATCH 5/6] worker_threads: take the claim before the fd check in closeIfUnclaimed A detached handle transfers fd -1. Its claim id stayed in the native set because closeIfUnclaimed returned on the fd before it took the claim. --- src/js/node/worker_threads.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/js/node/worker_threads.ts b/src/js/node/worker_threads.ts index 8030a84d74e6..b0b40107e19a 100644 --- a/src/js/node/worker_threads.ts +++ b/src/js/node/worker_threads.ts @@ -372,8 +372,9 @@ function isJSTransferableMarker(value: object): boolean { // The first thread to take a marker's claim id owns its fd, so one fd is never closed or restored twice. function closeIfUnclaimed(data: unknown, claim: number) { + if (!_claimJSTransferable(claim)) return; const fd = (data as any)?.fd; - if (typeof fd !== "number" || fd < 0 || !_claimJSTransferable(claim)) return; + if (typeof fd !== "number" || fd < 0) return; try { require("node:fs").closeSync(fd); } catch { From 805e28eb50cf4704eb19f846120162f29dc7f093 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 20 Sep 2026 01:25:30 +0000 Subject: [PATCH 6/6] ci: retrigger