diff --git a/doc/api/ffi.md b/doc/api/ffi.md index 29ff34015a7..342acfbec50 100644 --- a/doc/api/ffi.md +++ b/doc/api/ffi.md @@ -494,6 +494,14 @@ Closing the owning library or unregistering the currently executing callback from inside the callback is unsupported and dangerous. Doing so may crash the process, produce incorrect output, or corrupt memory. +If the thread running a callback is stopped while the callback executes, for +example by `worker.terminate()`, by `process.exit()` in a Worker, or by the +main thread exiting, only that thread stops. The callback returns to native +code without a value: non-void return values are zero-initialized, so native +code receives `0`, `false`, or a null pointer. Native code that does not +handle such a value, for example by dereferencing a returned null pointer, can +crash the process. + ### `library.unregisterCallback(pointer)` * `pointer` {bigint} diff --git a/src/node_ffi.cc b/src/node_ffi.cc index 732657a368e..e179cf811e1 100644 --- a/src/node_ffi.cc +++ b/src/node_ffi.cc @@ -766,6 +766,16 @@ void DynamicLibrary::InvokeCallback(ffi_cif* cif, MaybeLocal result = callback->Call( context, Undefined(isolate), expected_args, callback_args.data()); + // Termination (worker.terminate(), process.exit() in a Worker, or + // environment teardown) is not an exception thrown by the callback. + // Return a zeroed result and let the caller unwind. + if (try_catch.HasTerminated()) { + if (ret != nullptr && cb->return_type->size > 0) { + std::memset(ret, 0, GetFFIReturnValueStorageSize(cb->return_type)); + } + return; + } + // Handle exceptions by crashing (can't propagate across FFI boundary) if (try_catch.HasCaught()) { FPrintF(stderr, "Callbacks cannot throw an exception\n"); diff --git a/test/ffi/ffi-callback-test-common.js b/test/ffi/ffi-callback-test-common.js index 9772643bbdd..4ec87441a60 100644 --- a/test/ffi/ffi-callback-test-common.js +++ b/test/ffi/ffi-callback-test-common.js @@ -42,4 +42,5 @@ functions.call_int_callback(callback, 21);`, module.exports = { assertAborts, assertCallbackAborts, + spawnAbortingChild, }; diff --git a/test/ffi/test-ffi-callback-worker-termination.js b/test/ffi/test-ffi-callback-worker-termination.js new file mode 100644 index 00000000000..54500bc604a --- /dev/null +++ b/test/ffi/test-ffi-callback-worker-termination.js @@ -0,0 +1,54 @@ +'use strict'; +const common = require('../common'); +common.skipIfFFIMissing(); +const assert = require('node:assert'); +const { test } = require('node:test'); +const { spawnAbortingChild } = require('./ffi-callback-test-common'); +const { libraryPath } = require('./ffi-test-common'); + +// Stopping a Worker while it is inside an FFI callback terminates execution. +// That must stop only the Worker instead of aborting the process. +// The Worker must not load test/common: its 'exit' handler would throw from +// inside the callback on process.exit(), which is a real exception. +function runWorker(mode) { + const workerSource = ` +const { parentPort, workerData } = require('node:worker_threads'); +const ffi = require('node:ffi'); +const { lib, functions } = ffi.dlopen(${JSON.stringify(libraryPath)}, { + call_int_callback: { arguments: ['pointer', 'i32'], return: 'i32' }, +}); +const callback = lib.registerCallback( + { arguments: ['i32'], return: 'i32' }, + () => { + if (workerData === 'exit') process.exit(0); + parentPort.postMessage('in callback'); + for (;;); + }, +); +functions.call_int_callback(callback, 21); +`; + return spawnAbortingChild(`'use strict'; +const { Worker } = require('node:worker_threads'); +const worker = new Worker(${JSON.stringify(workerSource)}, { + eval: true, + workerData: ${JSON.stringify(mode)}, +}); +worker.on('message', () => { + if (${JSON.stringify(mode)} === 'shutdown') process.exit(0); + worker.terminate(); +}); +worker.on('exit', (code) => console.log('worker exited with code ' + code));`); +} + +for (const [mode, stdout] of [ + ['shutdown', ''], + ['terminate', 'worker exited with code 1\n'], + ['exit', 'worker exited with code 0\n'], +]) { + test(`stopping a Worker inside a callback (${mode}) does not abort`, () => { + const { status, signal, stdout: actual, stderr } = runWorker(mode); + assert.strictEqual(status, 0, `signal: ${signal}\nstderr: ${stderr}`); + assert.strictEqual(actual, stdout); + assert.doesNotMatch(stderr, /Callbacks cannot throw an exception/); + }); +}