From c5548d360ce932915717301bd5c49a4805397344 Mon Sep 17 00:00:00 2001 From: Max H Fisher Date: Wed, 22 Jul 2026 16:07:43 -0400 Subject: [PATCH 1/4] src: add SetAbortHandler Signed-off-by: Max H Fisher --- src/node.h | 10 ++++++ src/node_errors.cc | 20 ++++++++++++ src/util.h | 7 +++-- test/abort/test-addon-abort-handler.js | 4 +++ test/addons/abort-handler/binding.cc | 18 +++++++++++ test/addons/abort-handler/binding.gyp | 9 ++++++ test/addons/abort-handler/test.js | 43 ++++++++++++++++++++++++++ test/cctest/test_environment.cc | 40 ++++++++++++++++++++++++ 8 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 test/abort/test-addon-abort-handler.js create mode 100644 test/addons/abort-handler/binding.cc create mode 100644 test/addons/abort-handler/binding.gyp create mode 100644 test/addons/abort-handler/test.js diff --git a/src/node.h b/src/node.h index 32a9806ae84803..d9596f8082a2f0 100644 --- a/src/node.h +++ b/src/node.h @@ -844,6 +844,16 @@ NODE_EXTERN void SetProcessExitHandler( std::function&& handler); NODE_EXTERN void DefaultProcessExitHandler(Environment* env, int exit_code); +// Sets a process-global handler invoked when Node.js programmatically aborts +// (the ABORT() macro: failed CHECK/DCHECK, UNREACHABLE(), node::Assert(), and +// OnFatalError()). The handler may return, in which case Node.js still +// terminates the process (via abort()/_exit), or it may terminate the +// process itself. The default handler writes the native and JavaScript +// backtraces to stderr. Passing nullptr restores the default handler. This is +// process-global and may be invoked before any Isolate or Environment exists. +using AbortHandler = void (*)(); +NODE_EXTERN void SetAbortHandler(AbortHandler handler); + // This may return nullptr if context is not associated with a Node instance. NODE_EXTERN Environment* GetCurrentEnvironment(v8::Local context); NODE_EXTERN IsolateData* GetEnvironmentIsolateData(Environment* env); diff --git a/src/node_errors.cc b/src/node_errors.cc index 63db97f6a56db0..a0ef0b37ab724f 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -393,6 +393,26 @@ void AppendExceptionLine(Environment* env, .FromMaybe(false)); } +namespace { +// Default handler: reproduces previous ABORT() output (native + JS backtrace to +// stderr). Termination is NOT done here. The ABORT() macro backstops it. +void DefaultAbortHandler() { + DumpNativeBacktrace(stderr); + DumpJavaScriptBacktrace(stderr); + fflush(stderr); +} +// Constant-initialized, so this is valid from load time, safe even for a CHECK() +// during early startup, before any SetAbortHandler call. +AbortHandler g_abort_handler = DefaultAbortHandler; +} // namespace + +void SetAbortHandler(AbortHandler handler) { + g_abort_handler = handler ? handler : DefaultAbortHandler; +} +AbortHandler GetAbortHandler() { + return g_abort_handler; +} + void Assert(const AssertionInfo& info) { std::string name = GetHumanReadableProcessName(); diff --git a/src/util.h b/src/util.h index 48305bfdc13143..e61ef9b4b4b15b 100644 --- a/src/util.h +++ b/src/util.h @@ -126,6 +126,9 @@ void NODE_EXTERN_PRIVATE Assert(const AssertionInfo& info); void DumpNativeBacktrace(FILE* fp); void DumpJavaScriptBacktrace(FILE* fp); +// Returns the currently installed abort handler which is never null. +AbortHandler GetAbortHandler(); + // Windows 8+ does not like abort() in Release mode #ifdef _WIN32 #define ABORT_NO_BACKTRACE() _exit(static_cast(node::ExitCode::kAbort)) @@ -142,9 +145,7 @@ void DumpJavaScriptBacktrace(FILE* fp); // backtrace is correct. #define ABORT() \ do { \ - node::DumpNativeBacktrace(stderr); \ - node::DumpJavaScriptBacktrace(stderr); \ - fflush(stderr); \ + node::GetAbortHandler()(); \ ABORT_NO_BACKTRACE(); \ } while (0) diff --git a/test/abort/test-addon-abort-handler.js b/test/abort/test-addon-abort-handler.js new file mode 100644 index 00000000000000..86fcb8914f7cba --- /dev/null +++ b/test/abort/test-addon-abort-handler.js @@ -0,0 +1,4 @@ +'use strict'; +require('../common'); +process.env.ALLOW_CRASHES = true; +require('../addons/abort-handler/test'); diff --git a/test/addons/abort-handler/binding.cc b/test/addons/abort-handler/binding.cc new file mode 100644 index 00000000000000..9004f5cd1f50b2 --- /dev/null +++ b/test/addons/abort-handler/binding.cc @@ -0,0 +1,18 @@ +#include +#include +#include + +namespace { +void TestAbortHandler() { + fputs("CUSTOM_ABORT_HANDLER_RAN\n", stderr); + fflush(stderr); +} + +void InstallAbortHandler(const v8::FunctionCallbackInfo&) { + node::SetAbortHandler(TestAbortHandler); +} +} // namespace + +NODE_MODULE_INIT() { + NODE_SET_METHOD(exports, "installAbortHandler", InstallAbortHandler); +} diff --git a/test/addons/abort-handler/binding.gyp b/test/addons/abort-handler/binding.gyp new file mode 100644 index 00000000000000..55fbe7050f18e4 --- /dev/null +++ b/test/addons/abort-handler/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.cc' ], + 'includes': ['../common.gypi'], + } + ] +} diff --git a/test/addons/abort-handler/test.js b/test/addons/abort-handler/test.js new file mode 100644 index 00000000000000..8659d37b8e2739 --- /dev/null +++ b/test/addons/abort-handler/test.js @@ -0,0 +1,43 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const bindingPath = path.resolve( + __dirname, 'build', common.buildType, 'binding.node'); + +if (!fs.existsSync(bindingPath)) + common.skip('binding not built yet'); + +if (process.argv[2] === 'child') { + const binding = require(bindingPath); + binding.installAbortHandler(); + process.abort(); + return; +} + +// We do not want to generate core files / actually crash the process when +// running this test as a regular addon test. It is also required as an +// abort test with ALLOW_CRASHES set (see ../../abort/test-addon-abort-handler.js). +if (!process.env.ALLOW_CRASHES) + common.skip('test needs ALLOW_CRASHES to spawn a crashing child'); + +const result = spawnSync(process.execPath, [__filename, 'child']); + +const stderr = result.stderr.toString(); +assert.ok( + stderr.includes('CUSTOM_ABORT_HANDLER_RAN'), + `Expected custom abort handler marker in stderr, got:\n${stderr}`); +assert.ok( + !stderr.includes('Native stack trace'), + `Expected the custom handler to replace the default dump, got:\n${stderr}`); + +if (common.isWindows) { + assert.strictEqual(result.status, 134); + assert.strictEqual(result.signal, null); +} else { + assert.strictEqual(result.status, null); + assert.strictEqual(result.signal, 'SIGABRT'); +} diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 59c71835499e41..136e8ad6abac4a 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -1201,3 +1201,43 @@ TEST_F(EnvironmentTest, LoadEnvironmentWithCallbackWithESModule) { printf("Frame: %s\n", *frame_str); EXPECT_EQ(frame_str.ToString(), " at embedded:esm.mjs:3:15"); } + +namespace { +void CustomAbortHandlerForContractTest() {} + +bool abort_handler_dispatch_flag = false; +void AbortHandlerThatSetsDispatchFlag() { + abort_handler_dispatch_flag = true; +} +} // namespace + +TEST(AbortHandlerTest, DefaultIsNonNullAndSetAbortHandlerRoundTrips) { + node::AbortHandler old = node::GetAbortHandler(); + + // There should always be a non-null default handler installed. + EXPECT_NE(node::GetAbortHandler(), nullptr); + + node::SetAbortHandler(CustomAbortHandlerForContractTest); + EXPECT_EQ(node::GetAbortHandler(), CustomAbortHandlerForContractTest); + + node::SetAbortHandler(nullptr); + EXPECT_NE(node::GetAbortHandler(), nullptr); + EXPECT_NE(node::GetAbortHandler(), CustomAbortHandlerForContractTest); + + node::SetAbortHandler(old); +} + +TEST(AbortHandlerTest, InstalledHandlerIsInvokedWhenCalled) { + node::AbortHandler old = node::GetAbortHandler(); + abort_handler_dispatch_flag = false; + + node::SetAbortHandler(AbortHandlerThatSetsDispatchFlag); + node::AbortHandler h = node::GetAbortHandler(); + // Fail cleanly (instead of crashing on a null call) if the handler wasn't + // actually installed. + ASSERT_NE(h, nullptr); + h(); + EXPECT_TRUE(abort_handler_dispatch_flag); + + node::SetAbortHandler(old); +} From 0684453b632002dc2f94281ba8b8be36bf5853c7 Mon Sep 17 00:00:00 2001 From: Max H Fisher Date: Wed, 22 Jul 2026 17:48:50 -0400 Subject: [PATCH 2/4] fix: fix lint --- src/node_errors.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_errors.cc b/src/node_errors.cc index a0ef0b37ab724f..98d34b7db7eb02 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -401,8 +401,8 @@ void DefaultAbortHandler() { DumpJavaScriptBacktrace(stderr); fflush(stderr); } -// Constant-initialized, so this is valid from load time, safe even for a CHECK() -// during early startup, before any SetAbortHandler call. +// Constant-initialized, so this is valid from load time, safe even for a +// CHECK() during early startup, before any SetAbortHandler call. AbortHandler g_abort_handler = DefaultAbortHandler; } // namespace From 309902e69b6484556ee10ba6663c2e52ab8017dc Mon Sep 17 00:00:00 2001 From: Max H Fisher Date: Thu, 23 Jul 2026 17:16:39 -0400 Subject: [PATCH 3/4] fix: respond to PR comments Signed-off-by: Max H Fisher --- src/node.h | 13 ++++---- src/node_errors.cc | 16 +++++---- src/util.h | 5 +-- test/abort/test-addon-abort-handler.js | 4 --- test/addons/abort-handler/binding.cc | 2 +- test/addons/abort-handler/test.js | 46 ++++++++++++++------------ test/cctest/test_environment.cc | 14 ++++++-- 7 files changed, 55 insertions(+), 45 deletions(-) delete mode 100644 test/abort/test-addon-abort-handler.js diff --git a/src/node.h b/src/node.h index d9596f8082a2f0..471e4e84867090 100644 --- a/src/node.h +++ b/src/node.h @@ -844,14 +844,13 @@ NODE_EXTERN void SetProcessExitHandler( std::function&& handler); NODE_EXTERN void DefaultProcessExitHandler(Environment* env, int exit_code); -// Sets a process-global handler invoked when Node.js programmatically aborts -// (the ABORT() macro: failed CHECK/DCHECK, UNREACHABLE(), node::Assert(), and -// OnFatalError()). The handler may return, in which case Node.js still -// terminates the process (via abort()/_exit), or it may terminate the -// process itself. The default handler writes the native and JavaScript -// backtraces to stderr. Passing nullptr restores the default handler. This is +// Sets a process-global handler invoked when Node.js programmatically aborts. A +// message describing the reason for the abort may or may not be passed as a +// parameter to the handler. The handler should not return, but node will ensure +// that the process exits after the handler is called regardless of whether or +// not it returns. Passing nullptr restores the default handler. This is // process-global and may be invoked before any Isolate or Environment exists. -using AbortHandler = void (*)(); +using AbortHandler = void (*)(const char* message); NODE_EXTERN void SetAbortHandler(AbortHandler handler); // This may return nullptr if context is not associated with a Node instance. diff --git a/src/node_errors.cc b/src/node_errors.cc index 98d34b7db7eb02..8620edd5b046f4 100644 --- a/src/node_errors.cc +++ b/src/node_errors.cc @@ -394,12 +394,16 @@ void AppendExceptionLine(Environment* env, } namespace { -// Default handler: reproduces previous ABORT() output (native + JS backtrace to -// stderr). Termination is NOT done here. The ABORT() macro backstops it. -void DefaultAbortHandler() { +// Default handler: Dumps native + JS backtraces to stderr and exits. This +// indirectly calls backtrace so it can not be marked as [[noreturn]] (see the +// comment on node::Assert() below). `message` is ignored because the +// assertion/fatal-error message, if any, is already printed to stderr by the +// caller (Assert()/OnFatalError()) before this handler runs. +void DefaultAbortHandler(const char* /*message*/) { DumpNativeBacktrace(stderr); DumpJavaScriptBacktrace(stderr); fflush(stderr); + ABORT_NO_BACKTRACE(); } // Constant-initialized, so this is valid from load time, safe even for a // CHECK() during early startup, before any SetAbortHandler call. @@ -426,7 +430,7 @@ void Assert(const AssertionInfo& info) { info.message); fflush(stderr); - ABORT(); + ABORT_WITH_MESSAGE(info.message); } enum class EnhanceFatalException { kEnhance, kDontEnhance }; @@ -604,7 +608,7 @@ static void ReportFatalException(Environment* env, } fflush(stderr); - ABORT(); + ABORT_WITH_MESSAGE(message); } void OOMErrorHandler(const char* location, const v8::OOMDetails& details) { @@ -640,7 +644,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details) { } fflush(stderr); - ABORT(); + ABORT_WITH_MESSAGE(message); } v8::ModifyCodeGenerationFromStringsResult ModifyCodeGenerationFromStrings( diff --git a/src/util.h b/src/util.h index e61ef9b4b4b15b..83f1a680d38dfd 100644 --- a/src/util.h +++ b/src/util.h @@ -143,9 +143,10 @@ AbortHandler GetAbortHandler(); // correct backtracing. // `ABORT` must be a macro and not a [[noreturn]] function to make sure the // backtrace is correct. -#define ABORT() \ +#define ABORT() ABORT_WITH_MESSAGE(nullptr) +#define ABORT_WITH_MESSAGE(message) \ do { \ - node::GetAbortHandler()(); \ + node::GetAbortHandler()(message); \ ABORT_NO_BACKTRACE(); \ } while (0) diff --git a/test/abort/test-addon-abort-handler.js b/test/abort/test-addon-abort-handler.js deleted file mode 100644 index 86fcb8914f7cba..00000000000000 --- a/test/abort/test-addon-abort-handler.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict'; -require('../common'); -process.env.ALLOW_CRASHES = true; -require('../addons/abort-handler/test'); diff --git a/test/addons/abort-handler/binding.cc b/test/addons/abort-handler/binding.cc index 9004f5cd1f50b2..aca9cfb3701e0d 100644 --- a/test/addons/abort-handler/binding.cc +++ b/test/addons/abort-handler/binding.cc @@ -3,7 +3,7 @@ #include namespace { -void TestAbortHandler() { +void TestAbortHandler(const char* /*message*/) { fputs("CUSTOM_ABORT_HANDLER_RAN\n", stderr); fflush(stderr); } diff --git a/test/addons/abort-handler/test.js b/test/addons/abort-handler/test.js index 8659d37b8e2739..4c05046c92b16c 100644 --- a/test/addons/abort-handler/test.js +++ b/test/addons/abort-handler/test.js @@ -3,7 +3,7 @@ const common = require('../../common'); const assert = require('assert'); const fs = require('fs'); const path = require('path'); -const { spawnSync } = require('child_process'); +const { exec } = require('child_process'); const bindingPath = path.resolve( __dirname, 'build', common.buildType, 'binding.node'); @@ -18,26 +18,28 @@ if (process.argv[2] === 'child') { return; } -// We do not want to generate core files / actually crash the process when -// running this test as a regular addon test. It is also required as an -// abort test with ALLOW_CRASHES set (see ../../abort/test-addon-abort-handler.js). -if (!process.env.ALLOW_CRASHES) - common.skip('test needs ALLOW_CRASHES to spawn a crashing child'); - -const result = spawnSync(process.execPath, [__filename, 'child']); +const escapedArgs = + common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`; +if (!common.isWindows) { + // Do not create core files, as it can take a lot of disk space on + // continuous testing and developers' machines. + escapedArgs[0] = 'ulimit -c 0 && ' + escapedArgs[0]; +} -const stderr = result.stderr.toString(); -assert.ok( - stderr.includes('CUSTOM_ABORT_HANDLER_RAN'), - `Expected custom abort handler marker in stderr, got:\n${stderr}`); -assert.ok( - !stderr.includes('Native stack trace'), - `Expected the custom handler to replace the default dump, got:\n${stderr}`); +exec(...escapedArgs, common.mustCall((err, stdout, stderr) => { + assert.ok( + stderr.includes('CUSTOM_ABORT_HANDLER_RAN'), + `Expected custom abort handler marker in stderr, got:\n${stderr}`); + assert.ok( + !stderr.includes('Native stack trace'), + `Expected the custom handler to replace the default dump, got:\n${stderr}`); -if (common.isWindows) { - assert.strictEqual(result.status, 134); - assert.strictEqual(result.signal, null); -} else { - assert.strictEqual(result.status, null); - assert.strictEqual(result.signal, 'SIGABRT'); -} + // The child aborts. Whether that surfaces as the SIGABRT signal or as exit + // code 134 depends on shell wrapping: the `ulimit -c 0 && ...` prefix makes + // /bin/sh wait on (rather than exec-replace itself with) the node grandchild, + // so sh reports the aborted grandchild as a normal exit with code 134. + // common.nodeProcessAborted() accepts both forms. + assert.ok( + err && common.nodeProcessAborted(err.code, err.signal), + `Expected the child to abort, got code=${err?.code} signal=${err?.signal}`); +})); diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 136e8ad6abac4a..5df38c7712a3f3 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -1203,11 +1203,13 @@ TEST_F(EnvironmentTest, LoadEnvironmentWithCallbackWithESModule) { } namespace { -void CustomAbortHandlerForContractTest() {} +void CustomAbortHandlerForContractTest(const char* message) {} bool abort_handler_dispatch_flag = false; -void AbortHandlerThatSetsDispatchFlag() { +const char* abort_handler_received_message = nullptr; +void AbortHandlerThatSetsDispatchFlag(const char* message) { abort_handler_dispatch_flag = true; + abort_handler_received_message = message; } } // namespace @@ -1230,14 +1232,20 @@ TEST(AbortHandlerTest, DefaultIsNonNullAndSetAbortHandlerRoundTrips) { TEST(AbortHandlerTest, InstalledHandlerIsInvokedWhenCalled) { node::AbortHandler old = node::GetAbortHandler(); abort_handler_dispatch_flag = false; + abort_handler_received_message = nullptr; node::SetAbortHandler(AbortHandlerThatSetsDispatchFlag); node::AbortHandler h = node::GetAbortHandler(); // Fail cleanly (instead of crashing on a null call) if the handler wasn't // actually installed. ASSERT_NE(h, nullptr); - h(); + + // Dispatch through the public GetAbortHandler() accessor directly (not via + // the ABORT() macro, so nothing terminates), and verify the message is + // passed through unchanged. + node::GetAbortHandler()("some-test-message"); EXPECT_TRUE(abort_handler_dispatch_flag); + EXPECT_STREQ(abort_handler_received_message, "some-test-message"); node::SetAbortHandler(old); } From 71e66f86fdf582d833465de67fd0411f104dba02 Mon Sep 17 00:00:00 2001 From: Max Fisher <66274614+maxhfisher@users.noreply.github.com> Date: Fri, 24 Jul 2026 16:58:44 -0400 Subject: [PATCH 4/4] fix: Update default abort message Co-authored-by: Chengzhong Wu --- src/util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util.h b/src/util.h index 83f1a680d38dfd..e91d5651907dd7 100644 --- a/src/util.h +++ b/src/util.h @@ -143,7 +143,7 @@ AbortHandler GetAbortHandler(); // correct backtracing. // `ABORT` must be a macro and not a [[noreturn]] function to make sure the // backtrace is correct. -#define ABORT() ABORT_WITH_MESSAGE(nullptr) +#define ABORT() ABORT_WITH_MESSAGE(__FILE__ ":" STRINGIFY(__LINE__)) #define ABORT_WITH_MESSAGE(message) \ do { \ node::GetAbortHandler()(message); \