src,lib: make ^C print a JS stack trace#29207

legendecas · 2019-08-19T17:20:15Z

If terminating the process with ctrl-c / SIGINT, prints a JS stack trace leading up to the currently executing code.

The feature would be enabled under option --trace-sigint.

Conditions of no stacktrace on sigint:

has (an) active sigint listener(s);
main thread is idle (i.e. uv polling), a message instead of stacktrace would be printed.

Related: #24937

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

lib/internal/main/run_main_module.js

mscdex · 2019-08-19T17:53:32Z

Maybe we should only print a stack trace if stdin is a TTY (especially if it's meant to only happen when pressing ctrl-c)?

lib/internal/main/run_main_module.js

src/node_watchdog.cc

bnoordhuis

Left some comments but nice work!

src/node_watchdog.cc

addaleax

report: is the subsystem for the node-report feature, i.e. node --experimental-report … this touches a related topic, but I think it might be confusing to name it that way

src/node_watchdog.h

src/node_watchdog.cc

lib/internal/main/run_main_module.js

legendecas · 2019-08-24T13:32:20Z

report: is the subsystem for the node-report feature, i.e. node --experimental-report … this touches a related topic, but I think it might be confusing to name it that way

Yes, this work doesn't touch report specifically in codebase. Yet I don't find a more proper subsystem here https://github.com/nodejs/core-validate-commit/blob/master/lib/rules/subsystem.js . Since both src and lib are touched in this work, neither of those one might be appropriate IMO.

addaleax · 2019-08-24T13:33:28Z

@legendecas You can also use src,lib: .... 🙂 Honestly, I don’t have a better idea than that either.

lib/internal/bootstrap/pre_execution.js

legendecas · 2019-08-24T17:08:10Z

I think this PR is out of WIP, ready to be reviewed thoroughly :)

src/node_watchdog.h

nodejs-github-bot · 2019-08-25T14:54:14Z

CI: https://ci.nodejs.org/job/node-test-pull-request/25228/

addaleax · 2019-08-25T14:54:58Z

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1980/

addaleax · 2019-08-25T15:04:11Z

@legendecas Looks like there are still issues with the postmortem tests…

legendecas · 2019-08-25T16:45:35Z

@legendecas Looks like there are still issues with the postmortem tests…

I'll try to fix it :P

lib/internal/bootstrap/pre_execution.js

lib/internal/watchdog.js

Fishrock123 · 2020-01-14T19:50:14Z

I don't have any substantive opinions for this.

legendecas · 2020-01-17T02:56:17Z

Since there are no objections anymore and multiple approves, I'd tag this as author-ready.

If terminating the process with ctrl-c / SIGINT, prints a JS stacktrace leading up to the currently executing code. The feature would be enabled under option --trace-sigint. Conditions of no stacktrace on sigint: - has (an) active sigint listener(s); - main thread is idle (i.e. uv polling), a message instead of stacktrace would be printed.

legendecas · 2020-01-25T15:18:31Z

Rebased to resolve conflicts.

nodejs-github-bot · 2020-01-26T12:04:58Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28681/

If terminating the process with ctrl-c / SIGINT, prints a JS stacktrace leading up to the currently executing code. The feature would be enabled under option `--trace-sigint`. Conditions of no stacktrace on sigint: - has (an) active sigint listener(s); - main thread is idle (i.e. uv polling), a message instead of stacktrace would be printed. PR-URL: #29207 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Christopher Hiller <[email protected]> Reviewed-By: Rich Trott <[email protected]>

legendecas · 2020-01-28T05:53:59Z

Landed with 7b7e7bd 🎉

ruyadorno · 2020-01-28T14:51:36Z

/cc @nodejs/tooling this landed 🎉 yay! we might want to talk about it in the next tooling WG meeting

If terminating the process with ctrl-c / SIGINT, prints a JS stacktrace leading up to the currently executing code. The feature would be enabled under option `--trace-sigint`. Conditions of no stacktrace on sigint: - has (an) active sigint listener(s); - main thread is idle (i.e. uv polling), a message instead of stacktrace would be printed. PR-URL: #29207 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Christopher Hiller <[email protected]> Reviewed-By: Rich Trott <[email protected]>

If terminating the process with ctrl-c / SIGINT, prints a JS stacktrace leading up to the currently executing code. The feature would be enabled under option `--trace-sigint`. Conditions of no stacktrace on sigint: - has (an) active sigint listener(s); - main thread is idle (i.e. uv polling), a message instead of stacktrace would be printed. PR-URL: nodejs#29207 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Christopher Hiller <[email protected]> Reviewed-By: Rich Trott <[email protected]>

If terminating the process with ctrl-c / SIGINT, prints a JS stacktrace leading up to the currently executing code. The feature would be enabled under option `--trace-sigint`. Conditions of no stacktrace on sigint: - has (an) active sigint listener(s); - main thread is idle (i.e. uv polling), a message instead of stacktrace would be printed. PR-URL: #29207 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Christopher Hiller <[email protected]> Reviewed-By: Rich Trott <[email protected]>

orgads · 2025-01-30T14:26:01Z

Is it possible to extend this also for sigterm (with a separate flag)?

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Aug 19, 2019

mscdex reviewed Aug 19, 2019
View reviewed changes

lib/internal/main/run_main_module.js Outdated Show resolvedHide resolved

devsnek added the wip Issues and PRs that are still a work in progress. label Aug 19, 2019

addaleax reviewed Aug 20, 2019
View reviewed changes

lib/internal/main/run_main_module.js Outdated Show resolvedHide resolved
src/node_watchdog.cc Outdated Show resolvedHide resolved
src/node_watchdog.cc Outdated Show resolvedHide resolved
src/node_watchdog.cc Outdated Show resolvedHide resolved
src/node_watchdog.cc Outdated Show resolvedHide resolved

bnoordhuis reviewed Aug 20, 2019
View reviewed changes

jasnell reviewed Aug 20, 2019
View reviewed changes

src/node_watchdog.cc Outdated Show resolvedHide resolved

legendecas force-pushed the issue-24937 branch 3 times, most recently from 503aca6 to f175639Compare August 24, 2019 13:14

legendecas changed the title ~~cli: make ^C print a JS stack trace~~report: make ^C print a JS stack traceAug 24, 2019

addaleax approved these changes Aug 24, 2019
View reviewed changes

src/node_watchdog.h Outdated Show resolvedHide resolved
src/node_watchdog.h Outdated Show resolvedHide resolved
src/node_watchdog.cc Outdated Show resolvedHide resolved
lib/internal/main/run_main_module.js Outdated Show resolvedHide resolved

legendecas changed the title ~~report: make ^C print a JS stack trace~~src,lib: make ^C print a JS stack traceAug 24, 2019

legendecas force-pushed the issue-24937 branch 2 times, most recently from 3797e8e to 822e56aCompare August 24, 2019 14:13

addaleax approved these changes Aug 24, 2019
View reviewed changes

lib/internal/bootstrap/pre_execution.js Outdated Show resolvedHide resolved

legendecas force-pushed the issue-24937 branch 2 times, most recently from 124c8c3 to b7ccab2Compare August 24, 2019 16:44

addaleax removed the wip Issues and PRs that are still a work in progress. label Aug 24, 2019

addaleax reviewed Aug 24, 2019
View reviewed changes

src/node_watchdog.h Outdated Show resolvedHide resolved

mscdex reviewed Aug 25, 2019
View reviewed changes

lib/internal/bootstrap/pre_execution.js Outdated Show resolvedHide resolved

mscdex reviewed Aug 25, 2019
View reviewed changes

lib/internal/watchdog.js Outdated Show resolvedHide resolved

mscdex reviewed Aug 25, 2019
View reviewed changes

lib/internal/watchdog.js Outdated Show resolvedHide resolved

Fishrock123 removed their request for review January 14, 2020 19:49

legendecas added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 17, 2020

legendecas removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 25, 2020

legendecas force-pushed the issue-24937 branch from d5694b3 to df403e3Compare January 25, 2020 15:18

This comment has been minimized.
Sign in to view

legendecas added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 26, 2020

legendecas closed this Jan 28, 2020

legendecas deleted the issue-24937 branch January 28, 2020 05:54

bnoordhuis mentioned this pull request Jan 29, 2020
cli: make ^C print a JS stack trace #24937
Closed

codebytere mentioned this pull request Feb 17, 2020
v13.9.0 proposal #31837
Merged

This was referenced Apr 18, 2020
test: fix flaky test-trace-sigint-on-idle #31645
Closed
src: add missing namespace using statements in node_watchdog.h #32117
Closed

targos removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 25, 2020

targos mentioned this pull request May 2, 2020
v12.17.0 release proposal #33197
Merged

orgads mentioned this pull request Feb 2, 2025
Support printing backtrace also on SIGTERM #56879
Open

Uh oh!

src,lib: make ^C print a JS stack trace#29207

src,lib: make ^C print a JS stack trace #29207

Uh oh!

Conversation

legendecas commented Aug 19, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Uh oh!

mscdex commented Aug 19, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

legendecas commented Aug 24, 2019

Uh oh!

addaleax commented Aug 24, 2019

Uh oh!

Uh oh!

legendecas commented Aug 24, 2019

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 25, 2019

Uh oh!

addaleax commented Aug 25, 2019

Uh oh!

addaleax commented Aug 25, 2019

Uh oh!

legendecas commented Aug 25, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fishrock123 commented Jan 14, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

legendecas commented Jan 17, 2020

Uh oh!

legendecas commented Jan 25, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Jan 26, 2020

Uh oh!

legendecas commented Jan 28, 2020

Uh oh!

ruyadorno commented Jan 28, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orgads commented Jan 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

legendecas commented Aug 19, 2019•
edited
Loading

mscdex commented Aug 19, 2019•
edited
Loading

Fishrock123 commented Jan 14, 2020•
edited
Loading

ruyadorno commented Jan 28, 2020•
edited
Loading