From ec34b3efb0fdd2f3b6535ad477d068f36f967903 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 25 Sep 2026 16:46:11 +0200 Subject: [PATCH] test: split FFI call and callback coverage Isolate each callback abort scenario so timeouts identify the failing case. Consolidate callback GC coverage in the weakref test and disable core dumps for intentional aborts on POSIX. Signed-off-by: Filip Skokan Assisted-by: Codex --- test/ffi/ffi-callback-test-common.js | 45 ++++ test/ffi/ffi.status | 2 +- .../test-ffi-callback-abort-cross-thread.js | 28 ++ ...st-ffi-callback-abort-fractional-return.js | 9 + ...-ffi-callback-abort-out-of-range-return.js | 9 + test/ffi/test-ffi-callback-abort-promise.js | 9 + test/ffi/test-ffi-callback-abort-throw.js | 9 + test/ffi/test-ffi-callbacks.js | 84 ++++++ test/ffi/test-ffi-calls.js | 246 +----------------- test/ffi/test-ffi-weakref-calls.js | 22 ++ 10 files changed, 217 insertions(+), 246 deletions(-) create mode 100644 test/ffi/ffi-callback-test-common.js create mode 100644 test/ffi/test-ffi-callback-abort-cross-thread.js create mode 100644 test/ffi/test-ffi-callback-abort-fractional-return.js create mode 100644 test/ffi/test-ffi-callback-abort-out-of-range-return.js create mode 100644 test/ffi/test-ffi-callback-abort-promise.js create mode 100644 test/ffi/test-ffi-callback-abort-throw.js create mode 100644 test/ffi/test-ffi-callbacks.js diff --git a/test/ffi/ffi-callback-test-common.js b/test/ffi/ffi-callback-test-common.js new file mode 100644 index 00000000000..9772643bbdd --- /dev/null +++ b/test/ffi/ffi-callback-test-common.js @@ -0,0 +1,45 @@ +'use strict'; +const common = require('../common'); +const assert = require('node:assert'); +const { spawnSync } = require('node:child_process'); + +function spawnAbortingChild(source) { + const args = ['-e', source]; + if (common.isWindows) { + return spawnSync(process.execPath, args, { encoding: 'utf8' }); + } + + // Avoid writing core files for these intentional aborts. + return spawnSync('/bin/sh', [ + '-c', 'ulimit -c 0 && exec "$@"', + 'sh', process.execPath, ...args, + ], { encoding: 'utf8' }); +} + +function assertAborts(source, message) { + const { stderr, status, signal } = spawnAbortingChild(source); + assert.ok(common.nodeProcessAborted(status, signal), + `status: ${status}, signal: ${signal} +stderr: ${stderr}`); + assert.match(stderr, message); +} + +function assertCallbackAborts(callbackBody, message) { + assertAborts( + `'use strict'; +const ffi = require('node:ffi'); +const { fixtureSymbols, libraryPath } = require(${JSON.stringify(require.resolve('./ffi-test-common'))}); +const { lib, functions } = ffi.dlopen(libraryPath, fixtureSymbols); +const callback = lib.registerCallback( + { arguments: ['i32'], return: 'i32' }, + () => { ${callbackBody} }, +); +functions.call_int_callback(callback, 21);`, + message, + ); +} + +module.exports = { + assertAborts, + assertCallbackAborts, +}; diff --git a/test/ffi/ffi.status b/test/ffi/ffi.status index a5274bfaf4a..0cd3513c3cb 100644 --- a/test/ffi/ffi.status +++ b/test/ffi/ffi.status @@ -4,6 +4,6 @@ prefix ffi [$system==solaris] # Also applies to SmartOS # Bundled libffi callbacks crash on SmartOS. -test-ffi-calls: SKIP +test-ffi-callback*: SKIP test-ffi-shared-buffer: SKIP test-ffi-weakref-calls: SKIP diff --git a/test/ffi/test-ffi-callback-abort-cross-thread.js b/test/ffi/test-ffi-callback-abort-cross-thread.js new file mode 100644 index 00000000000..91ae44f493a --- /dev/null +++ b/test/ffi/test-ffi-callback-abort-cross-thread.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common'); +common.skipIfFFIMissing(); +const { test } = require('node:test'); +const { assertAborts } = require('./ffi-callback-test-common'); + +test('ffi aborts on cross-thread callback invocation', () => { + const workerSource = ` +const { workerData } = require('node:worker_threads'); +const ffi = require('node:ffi'); +const { fixtureSymbols, libraryPath } = require(${JSON.stringify(require.resolve('./ffi-test-common'))}); +const { functions } = ffi.dlopen(libraryPath, fixtureSymbols); +functions.call_int_callback(workerData, 21); +`; + assertAborts( + `'use strict'; +const { Worker } = require('node:worker_threads'); +const ffi = require('node:ffi'); +const { fixtureSymbols, libraryPath } = require(${JSON.stringify(require.resolve('./ffi-test-common'))}); +const { lib } = ffi.dlopen(libraryPath, fixtureSymbols); +const callback = lib.registerCallback( + { arguments: ['i32'], return: 'i32' }, + (value) => value * 2, +); +new Worker(${JSON.stringify(workerSource)}, { eval: true, workerData: callback });`, + /Callbacks can only be invoked on the system thread they were created on/, + ); +}); diff --git a/test/ffi/test-ffi-callback-abort-fractional-return.js b/test/ffi/test-ffi-callback-abort-fractional-return.js new file mode 100644 index 00000000000..c20a2e84a95 --- /dev/null +++ b/test/ffi/test-ffi-callback-abort-fractional-return.js @@ -0,0 +1,9 @@ +'use strict'; +const common = require('../common'); +common.skipIfFFIMissing(); +const { test } = require('node:test'); +const { assertCallbackAborts } = require('./ffi-callback-test-common'); + +test('ffi aborts on fractional callback return values', () => { + assertCallbackAborts('return 1.5;', /Callback returned invalid value for declared FFI type/); +}); diff --git a/test/ffi/test-ffi-callback-abort-out-of-range-return.js b/test/ffi/test-ffi-callback-abort-out-of-range-return.js new file mode 100644 index 00000000000..28dbe129792 --- /dev/null +++ b/test/ffi/test-ffi-callback-abort-out-of-range-return.js @@ -0,0 +1,9 @@ +'use strict'; +const common = require('../common'); +common.skipIfFFIMissing(); +const { test } = require('node:test'); +const { assertCallbackAborts } = require('./ffi-callback-test-common'); + +test('ffi aborts on out-of-range callback return values', () => { + assertCallbackAborts('return 2 ** 40;', /Callback returned invalid value for declared FFI type/); +}); diff --git a/test/ffi/test-ffi-callback-abort-promise.js b/test/ffi/test-ffi-callback-abort-promise.js new file mode 100644 index 00000000000..bbfa55236bd --- /dev/null +++ b/test/ffi/test-ffi-callback-abort-promise.js @@ -0,0 +1,9 @@ +'use strict'; +const common = require('../common'); +common.skipIfFFIMissing(); +const { test } = require('node:test'); +const { assertCallbackAborts } = require('./ffi-callback-test-common'); + +test('ffi aborts when a callback returns a promise', () => { + assertCallbackAborts('return Promise.resolve(1);', /Callbacks cannot return promises/); +}); diff --git a/test/ffi/test-ffi-callback-abort-throw.js b/test/ffi/test-ffi-callback-abort-throw.js new file mode 100644 index 00000000000..61113b39e01 --- /dev/null +++ b/test/ffi/test-ffi-callback-abort-throw.js @@ -0,0 +1,9 @@ +'use strict'; +const common = require('../common'); +common.skipIfFFIMissing(); +const { test } = require('node:test'); +const { assertCallbackAborts } = require('./ffi-callback-test-common'); + +test('ffi aborts when a callback throws', () => { + assertCallbackAborts('throw new Error("boom");', /Callbacks cannot throw an exception/); +}); diff --git a/test/ffi/test-ffi-callbacks.js b/test/ffi/test-ffi-callbacks.js new file mode 100644 index 00000000000..e402343bbc2 --- /dev/null +++ b/test/ffi/test-ffi-callbacks.js @@ -0,0 +1,84 @@ +'use strict'; +const common = require('../common'); +common.skipIfFFIMissing(); +const assert = require('node:assert'); +const { test } = require('node:test'); +const ffi = require('node:ffi'); +const { cString, fixtureSymbols, libraryPath } = require('./ffi-test-common'); + +function getLibrary() { + return ffi.dlopen(libraryPath, fixtureSymbols); +} + +test('ffi callbacks can be registered and invoked', () => { + const { lib, functions: symbols } = getLibrary(); + const seen = []; + const intCallback = lib.registerCallback( + { arguments: ['i32'], return: 'i32' }, + (value) => value * 2, + ); + const stringCallback = lib.registerCallback( + { arguments: ['pointer'], return: 'void' }, + (ptr) => seen.push(ffi.toString(ptr)), + ); + const binaryCallback = lib.registerCallback( + { arguments: ['i32', 'i32'], return: 'i32' }, + (a, b) => a + b, + ); + + try { + assert.strictEqual(symbols.call_int_callback(intCallback, 21), 42); + symbols.call_string_callback(stringCallback, cString('hello callback')); + assert.deepStrictEqual(seen, ['hello callback']); + assert.strictEqual(symbols.call_binary_int_callback(binaryCallback, 19, 23), 42); + + const nullPointerCallback = lib.registerCallback({ return: 'pointer' }, () => null); + const undefinedPointerCallback = lib.registerCallback({ return: 'pointer' }, () => undefined); + try { + assert.strictEqual(symbols.call_pointer_callback_is_null(nullPointerCallback), 1); + assert.strictEqual(symbols.call_pointer_callback_is_null(undefinedPointerCallback), 1); + } finally { + lib.unregisterCallback(nullPointerCallback); + lib.unregisterCallback(undefinedPointerCallback); + } + } finally { + lib.unregisterCallback(intCallback); + lib.unregisterCallback(stringCallback); + lib.unregisterCallback(binaryCallback); + lib.close(); + } +}); + +test('ffi callback ref and unref APIs work', () => { + const { lib, functions: symbols } = getLibrary(); + let called = false; + const values = []; + const voidCallback = lib.registerCallback(() => { + called = true; + }); + const countingCallback = lib.registerCallback( + { arguments: ['i32'], return: 'i32' }, + (value) => { + values.push(value); + return 0; + }, + ); + + try { + lib.unrefCallback(voidCallback); + lib.refCallback(voidCallback); + symbols.call_void_callback(voidCallback); + symbols.call_callback_multiple_times(countingCallback, 5); + + assert.strictEqual(called, true); + assert.deepStrictEqual(values, [0, 1, 2, 3, 4]); + + lib.unregisterCallback(voidCallback); + lib.unregisterCallback(countingCallback); + + assert.throws(() => lib.refCallback(voidCallback), /Callback not found/); + assert.throws(() => lib.unregisterCallback(-1n), /The first argument must be a non-negative bigint/); + } finally { + lib.close(); + } +}); diff --git a/test/ffi/test-ffi-calls.js b/test/ffi/test-ffi-calls.js index 3314c566b1c..bc3be0072db 100644 --- a/test/ffi/test-ffi-calls.js +++ b/test/ffi/test-ffi-calls.js @@ -1,10 +1,8 @@ -// Flags: --expose-gc --allow-natives-syntax +// Flags: --allow-natives-syntax 'use strict'; const common = require('../common'); common.skipIfFFIMissing(); -const { gcUntil } = require('../common/gc'); const assert = require('node:assert'); -const { spawnSync } = require('node:child_process'); const { test } = require('node:test'); const ffi = require('node:ffi'); const { cString, fixtureSymbols, libraryPath } = require('./ffi-test-common'); @@ -193,79 +191,6 @@ test('ffi global state helpers work', () => { } }); -test('ffi callbacks can be registered and invoked', () => { - const { lib, functions: symbols } = getLibrary(); - const seen = []; - const intCallback = lib.registerCallback( - { arguments: ['i32'], return: 'i32' }, - (value) => value * 2, - ); - const stringCallback = lib.registerCallback( - { arguments: ['pointer'], return: 'void' }, - (ptr) => seen.push(ffi.toString(ptr)), - ); - const binaryCallback = lib.registerCallback( - { arguments: ['i32', 'i32'], return: 'i32' }, - (a, b) => a + b, - ); - - try { - assert.strictEqual(symbols.call_int_callback(intCallback, 21), 42); - symbols.call_string_callback(stringCallback, cString('hello callback')); - assert.deepStrictEqual(seen, ['hello callback']); - assert.strictEqual(symbols.call_binary_int_callback(binaryCallback, 19, 23), 42); - - const nullPointerCallback = lib.registerCallback({ return: 'pointer' }, () => null); - const undefinedPointerCallback = lib.registerCallback({ return: 'pointer' }, () => undefined); - try { - assert.strictEqual(symbols.call_pointer_callback_is_null(nullPointerCallback), 1); - assert.strictEqual(symbols.call_pointer_callback_is_null(undefinedPointerCallback), 1); - } finally { - lib.unregisterCallback(nullPointerCallback); - lib.unregisterCallback(undefinedPointerCallback); - } - } finally { - lib.unregisterCallback(intCallback); - lib.unregisterCallback(stringCallback); - lib.unregisterCallback(binaryCallback); - lib.close(); - } -}); - -test('ffi callback ref and unref APIs work', () => { - const { lib, functions: symbols } = getLibrary(); - let called = false; - const values = []; - const voidCallback = lib.registerCallback(() => { - called = true; - }); - const countingCallback = lib.registerCallback( - { arguments: ['i32'], return: 'i32' }, - (value) => { - values.push(value); - return 0; - }, - ); - - try { - lib.unrefCallback(voidCallback); - lib.refCallback(voidCallback); - symbols.call_void_callback(voidCallback); - symbols.call_callback_multiple_times(countingCallback, 5); - - assert.strictEqual(called, true); - assert.deepStrictEqual(values, [0, 1, 2, 3, 4]); - - lib.unregisterCallback(voidCallback); - lib.unregisterCallback(countingCallback); - - assert.throws(() => lib.refCallback(voidCallback), /Callback not found/); - assert.throws(() => lib.unregisterCallback(-1n), /The first argument must be a non-negative bigint/); - } finally { - lib.close(); - } -}); - test('ffi validates invalid arguments', () => { const { lib, functions: symbols } = getLibrary(); try { @@ -310,172 +235,3 @@ test('ffi division helpers behave as expected', () => { lib.close(); } }); - -function assertInvalidCallbackReturnAborts(returnExpression) { - const { stderr, status, signal } = spawnSync(process.execPath, [ - '-e', - `'use strict'; -const ffi = require('node:ffi'); -const { fixtureSymbols, libraryPath } = require(${JSON.stringify(require.resolve('./ffi-test-common'))}); -const { lib, functions } = ffi.dlopen(libraryPath, fixtureSymbols); -const callback = lib.registerCallback( - { arguments: ['i32'], return: 'i32' }, - () => (${returnExpression}), -); -functions.call_int_callback(callback, 21);`, - ], { - encoding: 'utf8', - }); - - assert.ok(common.nodeProcessAborted(status, signal), - `status: ${status}, signal: ${signal} -stderr: ${stderr}`); - assert.match(stderr, /Callback returned invalid value for declared FFI type/); -} - -function assertInvalidCallbackBehaviorAborts(callbackBody, message) { - const { stderr, status, signal } = spawnSync(process.execPath, [ - '-e', - `'use strict'; -const ffi = require('node:ffi'); -const { fixtureSymbols, libraryPath } = require(${JSON.stringify(require.resolve('./ffi-test-common'))}); -const { lib, functions } = ffi.dlopen(libraryPath, fixtureSymbols); -const callback = lib.registerCallback( - { arguments: ['i32'], return: 'i32' }, - () => { ${callbackBody} }, -); -functions.call_int_callback(callback, 21);`, - ], { - encoding: 'utf8', - }); - - assert.ok(common.nodeProcessAborted(status, signal), - `status: ${status}, signal: ${signal} -stderr: ${stderr}`); - assert.ok(message.test(stderr), stderr); -} - -function assertCrossThreadCallbackAbort() { - const workerSource = ` -const { workerData } = require('node:worker_threads'); -const ffi = require('node:ffi'); -const { fixtureSymbols, libraryPath } = require(${JSON.stringify(require.resolve('./ffi-test-common'))}); -const { functions } = ffi.dlopen(libraryPath, fixtureSymbols); -functions.call_int_callback(workerData, 21); -`; - const { stderr, status, signal } = spawnSync(process.execPath, [ - '-e', - `'use strict'; -const { Worker } = require('node:worker_threads'); -const ffi = require('node:ffi'); -const { fixtureSymbols, libraryPath } = require(${JSON.stringify(require.resolve('./ffi-test-common'))}); -const { lib } = ffi.dlopen(libraryPath, fixtureSymbols); -const callback = lib.registerCallback( - { arguments: ['i32'], return: 'i32' }, - (value) => value * 2, -); -new Worker(${JSON.stringify(workerSource)}, { eval: true, workerData: callback });`, - ], { - encoding: 'utf8', - }); - - assert.ok(common.nodeProcessAborted(status, signal), - `status: ${status}, signal: ${signal} -stderr: ${stderr}`); - assert.match(stderr, /Callbacks can only be invoked on the system thread they were created on/); -} - -test('ffi aborts on invalid callback return values', () => { - assertInvalidCallbackReturnAborts('1.5'); - assertInvalidCallbackReturnAborts('2 ** 40'); -}); - -test('ffi aborts on invalid callback behavior', () => { - assertInvalidCallbackBehaviorAborts('throw new Error("boom");', /Callbacks cannot throw an exception/); - assertInvalidCallbackBehaviorAborts('return Promise.resolve(1);', /Callbacks cannot return promises/); -}); - -test('ffi aborts on cross-thread callback invocation', () => { - assertCrossThreadCallbackAbort(); -}); - -test('ffi unrefCallback releases callback function', async () => { - const { lib, functions: symbols } = getLibrary(); - try { - let callback = () => 1; - const ref = new WeakRef(callback); - const pointer = lib.registerCallback( - { arguments: ['i32'], return: 'i32' }, - callback, - ); - - lib.unrefCallback(pointer); - callback = null; - - await gcUntil('ffi unrefCallback releases callback function', () => { - return ref.deref() === undefined; - }); - - assert.strictEqual(symbols.call_int_callback(pointer, 21), 0); - lib.unregisterCallback(pointer); - } finally { - lib.close(); - } -}); - -test('ffi unrefCallback zero-fills narrow callback return', async () => { - const { lib, functions: symbols } = getLibrary(); - try { - let callback = () => 1; - const ref = new WeakRef(callback); - const pointer = lib.registerCallback( - { arguments: ['i8'], return: 'i8' }, - callback, - ); - - lib.unrefCallback(pointer); - callback = null; - - await gcUntil('ffi unrefCallback zero-fills narrow callback return', () => { - return ref.deref() === undefined; - }); - - assert.strictEqual(symbols.call_int8_callback(pointer, 21), 0); - lib.unregisterCallback(pointer); - } finally { - lib.close(); - } -}); - -test('ffi refCallback retains callback function', async () => { - const { lib } = getLibrary(); - try { - let callback = () => 1; - const ref = new WeakRef(callback); - const pointer = lib.registerCallback({ return: 'i32' }, callback); - - lib.unrefCallback(pointer); - lib.refCallback(pointer); - callback = null; - - for (let i = 0; i < 5; i++) { - await gcUntil('ffi refCallback retains callback function', () => true, 1); - assert.strictEqual(typeof ref.deref(), 'function'); - } - - lib.unregisterCallback(pointer); - } finally { - lib.close(); - } -}); - -test('closing a library invalidates callbacks', () => { - const { lib } = getLibrary(); - const callback = lib.registerCallback(() => {}); - - lib.close(); - - assert.throws(() => lib.unregisterCallback(callback), /Library is closed/); - assert.throws(() => lib.refCallback(callback), /Library is closed/); - assert.throws(() => lib.unrefCallback(callback), /Library is closed/); -}); diff --git a/test/ffi/test-ffi-weakref-calls.js b/test/ffi/test-ffi-weakref-calls.js index 432b39797f4..4daa26a9e35 100644 --- a/test/ffi/test-ffi-weakref-calls.js +++ b/test/ffi/test-ffi-weakref-calls.js @@ -31,6 +31,28 @@ test('ffi unrefCallback releases callback function', async (t) => { lib.unregisterCallback(pointer); }); +test('ffi unrefCallback zero-fills narrow callback return', async (t) => { + const { lib, functions: symbols } = ffi.dlopen(libraryPath, fixtureSymbols); + t.after(() => lib.close()); + + let callback = () => 1; + const ref = new WeakRef(callback); + const pointer = lib.registerCallback( + { arguments: ['i8'], return: 'i8' }, + callback, + ); + + lib.unrefCallback(pointer); + callback = null; + + await gcUntil('ffi unrefCallback zero-fills narrow callback return', () => { + return ref.deref() === undefined; + }); + + t.assert.strictEqual(symbols.call_int8_callback(pointer, 21), 0); + lib.unregisterCallback(pointer); +}); + test('ffi refCallback retains callback function', async (t) => { const { lib } = ffi.dlopen(libraryPath, fixtureSymbols); t.after(() => lib.close());