-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
cli: add --trace-exit cli option #30516
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
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 |
---|---|---|
|
@@ -940,6 +940,21 @@ void AsyncHooks::grow_async_ids_stack() { | |
uv_key_t Environment::thread_local_env = {}; | ||
|
||
void Environment::Exit(int exit_code) { | ||
if (options()->trace_exit) { | ||
HandleScope handle_scope(isolate()); | ||
|
||
if (is_main_thread()) { | ||
fprintf(stderr, "(node:%d) ", uv_os_getpid()); | ||
} else { | ||
fprintf(stderr, "(node:%d, thread:%llu) ", uv_os_getpid(), thread_id()); | ||
} | ||
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. Ditto, are this function called always from exceptions? |
||
|
||
fprintf( | ||
stderr, "WARNING: Exited the environment with code %d\n", exit_code); | ||
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. Why do we mark this as a warning and pipe to 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. Other traces were printed as warnings and piped to 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. Feel free to resolve this by yourself, I'm okay if this is a default behavior :) |
||
PrintStackTrace( | ||
isolate(), | ||
StackTrace::CurrentStackTrace(isolate(), 10, StackTrace::kDetailed)); | ||
legendecas marked this conversation as resolved.
Show resolved
Hide resolved
yorkie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
if (is_main_thread()) { | ||
stop_sub_worker_contexts(); | ||
DisposePlatform(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
'use strict'; | ||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const { promisify } = require('util'); | ||
const execFile = promisify(require('child_process').execFile); | ||
const { Worker, isMainThread, workerData } = require('worker_threads'); | ||
|
||
const variant = process.argv[process.argv.length - 1]; | ||
switch (true) { | ||
case variant === 'main-thread': { | ||
return; | ||
} | ||
case variant === 'main-thread-exit': { | ||
return process.exit(0); | ||
} | ||
case variant.startsWith('worker-thread'): { | ||
const worker = new Worker(__filename, { workerData: variant }); | ||
worker.on('error', common.mustNotCall()); | ||
worker.on('exit', common.mustCall((code) => { | ||
assert.strictEqual(code, 0); | ||
})); | ||
return; | ||
} | ||
case !isMainThread: { | ||
if (workerData === 'worker-thread-exit') { | ||
process.exit(0); | ||
} | ||
return; | ||
} | ||
} | ||
|
||
(async function() { | ||
for (const { execArgv, variant, warnings } of [ | ||
{ execArgv: ['--trace-exit'], variant: 'main-thread-exit', warnings: 1 }, | ||
{ execArgv: [], variant: 'main-thread-exit', warnings: 0 }, | ||
{ execArgv: ['--trace-exit'], variant: 'main-thread', warnings: 0 }, | ||
{ execArgv: [], variant: 'main-thread', warnings: 0 }, | ||
{ execArgv: ['--trace-exit'], variant: 'worker-thread-exit', warnings: 1 }, | ||
{ execArgv: [], variant: 'worker-thread-exit', warnings: 0 }, | ||
{ execArgv: ['--trace-exit'], variant: 'worker-thread', warnings: 0 }, | ||
{ execArgv: [], variant: 'worker-thread', warnings: 0 }, | ||
]) { | ||
const { stdout, stderr } = | ||
await execFile(process.execPath, [...execArgv, __filename, variant]); | ||
assert.strictEqual(stdout, ''); | ||
const actualWarnings = | ||
stderr.match(/WARNING: Exited the environment with code 0/g); | ||
if (warnings === 0) { | ||
assert.strictEqual(actualWarnings, null); | ||
return; | ||
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 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. Thank you for identifying this. Would you mind opening an issue for it (preferably with a snippet showing the bug if you have one)? It will get more attention as an issue that a comment here. |
||
} | ||
assert.strictEqual(actualWarnings.length, warnings); | ||
|
||
if (variant.startsWith('worker')) { | ||
const workerIds = stderr.match(/\(node:\d+, thread:\d+)/g); | ||
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 regular expression in invalid. It contains an unmatched 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. Thanks for reporting this! I've submitted a fix for this #33769. |
||
assert.strictEqual(workerIds.length, warnings); | ||
} | ||
} | ||
})().then(common.mustCall()); |
Uh oh!
There was an error while loading. Please reload this page.