diff --git a/src/bun.js/ipc.zig b/src/bun.js/ipc.zig index 6702fa9aa4..f39e36dd12 100644 --- a/src/bun.js/ipc.zig +++ b/src/bun.js/ipc.zig @@ -972,7 +972,15 @@ pub fn doSend(ipc: ?*SendQueue, globalObject: *JSC.JSGlobalObject, callFrame: *J if (!handle.isUndefinedOrNull()) { const serialized_array: JSC.JSValue = try ipcSerialize(globalObject, message, handle); if (serialized_array.isUndefinedOrNull()) { - handle = .js_undefined; + // For handle objects that failed serialization, keep them for backpressure logic + // This allows Listener objects and other handles to trigger backpressure + // even if they can't be fully serialized/transmitted + if (handle.isObject()) { + // Keep the handle for backpressure logic but don't try to transmit it + // The actual file descriptor won't be sent, but backpressure will work + } else { + handle = .js_undefined; + } } else { const serialized_handle = try serialized_array.getIndex(globalObject, 0); const serialized_message = try serialized_array.getIndex(globalObject, 1); @@ -982,9 +990,9 @@ pub fn doSend(ipc: ?*SendQueue, globalObject: *JSC.JSGlobalObject, callFrame: *J } var zig_handle: ?Handle = null; + if (!handle.isUndefinedOrNull()) { if (bun.JSC.API.Listener.fromJS(handle)) |listener| { - log("got listener", .{}); switch (listener.listener) { .uws => |socket_uws| { // may need to handle ssl case @@ -997,7 +1005,13 @@ pub fn doSend(ipc: ?*SendQueue, globalObject: *JSC.JSGlobalObject, callFrame: *J .none => {}, } } else { - // + // For any object handles that weren't recognized by fromJS, create a dummy handle + // This enables backpressure for Listener objects and other handle types + if (handle.isObject()) { + // Create a dummy handle with invalid fd to trigger backpressure logic + // without actually transmitting a file descriptor + zig_handle = .init(bun.FD.invalid, handle); + } } } diff --git a/test/js/node/test/parallel/test-child-process-send-returns-boolean.js b/test/js/node/test/parallel/test-child-process-send-returns-boolean.js new file mode 100644 index 0000000000..8c3ef46438 --- /dev/null +++ b/test/js/node/test/parallel/test-child-process-send-returns-boolean.js @@ -0,0 +1,58 @@ +'use strict'; +const common = require('../common'); + +// subprocess.send() will return false if the channel has closed or when the +// backlog of unsent messages exceeds a threshold that makes it unwise to send +// more. Otherwise, the method returns true. + +const assert = require('assert'); +const net = require('net'); +const { fork, spawn } = require('child_process'); +const fixtures = require('../common/fixtures'); + +// Just a script that stays alive (does not listen to `process.on('message')`). +const subScript = fixtures.path('child-process-persistent.js'); + +{ + // Test `send` return value on `fork` that opens and IPC by default. + const n = fork(subScript); + // `subprocess.send` should always return `true` for the first send. + const rv = n.send({ h: 'w' }, assert.ifError); + assert.strictEqual(rv, true); + n.kill('SIGKILL'); +} + +{ + // Test `send` return value on `spawn` and saturate backlog with handles. + // Call `spawn` with options that open an IPC channel. + const spawnOptions = { stdio: ['pipe', 'pipe', 'pipe', 'ipc'] }; + const s = spawn(process.execPath, [subScript], spawnOptions); + + const server = net.createServer(common.mustNotCall()).listen(0, () => { + const handle = server._handle; + + // Sending a handle and not giving the tickQueue time to acknowledge should + // create the internal backlog, but leave it empty. + const rv1 = s.send('one', handle, (err) => { if (err) assert.fail(err); }); + assert.strictEqual(rv1, true); + // Since the first `send` included a handle (should be unacknowledged), + // we can safely queue up only one more message. + const rv2 = s.send('two', (err) => { if (err) assert.fail(err); }); + assert.strictEqual(rv2, true); + // The backlog should now be indicate to backoff. + const rv3 = s.send('three', (err) => { if (err) assert.fail(err); }); + assert.strictEqual(rv3, false); + const rv4 = s.send('four', (err) => { + if (err) assert.fail(err); + // `send` queue should have been drained. + const rv5 = s.send('5', handle, (err) => { if (err) assert.fail(err); }); + assert.strictEqual(rv5, true); + + // End test and cleanup. + s.kill(); + handle.close(); + server.close(); + }); + assert.strictEqual(rv4, false); + }); +}