-
-
Notifications
You must be signed in to change notification settings - Fork 36.2k
src: add SetAbortHandler #64684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
src: add SetAbortHandler #64684
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -844,6 +844,16 @@ NODE_EXTERN void SetProcessExitHandler( | |||||
| std::function<void(Environment*, int)>&& 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 | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| // process itself. The default handler writes the native and JavaScript | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "writes native and JS backtrace" is an best effort behavior. It is not guaranteed and better not be documented as the public API contract. |
||||||
| // 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 (*)(); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to allow embedders/addons to persist the reason of the abort (like distinguishing multiple ABORT calls in a same function), it would be better to add a |
||||||
| 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<v8::Context> context); | ||||||
| NODE_EXTERN IsolateData* GetEnvironmentIsolateData(Environment* env); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -393,6 +393,26 @@ void AppendExceptionLine(Environment* env, | |
| .FromMaybe(false)); | ||
| } | ||
|
|
||
| namespace { | ||
| // Default handler: reproduces previous ABORT() output (native + JS backtrace to | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is weird to say "previous" in the document. Please just document the behavior straight. |
||
| // stderr). Termination is NOT done here. The ABORT() macro backstops it. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The API contract is that the abort handler should not return (even if the ABORT() macro ensures that the abort handler must not return). This default handler should also not return. |
||
| 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(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| 'use strict'; | ||
| require('../common'); | ||
| process.env.ALLOW_CRASHES = true; | ||
| require('../addons/abort-handler/test'); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This creates a dependency of a |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| #include <node.h> | ||
| #include <v8.h> | ||
| #include <cstdio> | ||
|
|
||
| namespace { | ||
| void TestAbortHandler() { | ||
| fputs("CUSTOM_ABORT_HANDLER_RAN\n", stderr); | ||
| fflush(stderr); | ||
| } | ||
|
|
||
| void InstallAbortHandler(const v8::FunctionCallbackInfo<v8::Value>&) { | ||
| node::SetAbortHandler(TestAbortHandler); | ||
| } | ||
| } // namespace | ||
|
|
||
| NODE_MODULE_INIT() { | ||
| NODE_SET_METHOD(exports, "installAbortHandler", InstallAbortHandler); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| 'targets': [ | ||
| { | ||
| 'target_name': 'binding', | ||
| 'sources': [ 'binding.cc' ], | ||
| 'includes': ['../common.gypi'], | ||
| } | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are internal details. I don' think this should be documented in the public api.