Skip to content
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

console: don't swallow stack overflow #19423

Closed
wants to merge 2 commits into from
Closed

console: don't swallow stack overflow #19423

wants to merge 2 commits into from

Conversation

dbkaplun
Copy link
Contributor

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

console.log unintentionally swallows stack overflows from V8. MAX_STACK_MESSAGE is never set as a is not called. The original code was intentionally platform-agnostic, so this logic avoids platform-specific logic as well.

Fixes test/parallel/test-console-no-swallow-stack-exceeded.js
@nodejs-github-bot nodejs-github-bot added console Issues and PRs related to the console subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Mar 18, 2018
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although I’m not sure how this affects behaviour?

Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for fixing this!

@trivikr
Copy link
Member

trivikr commented Mar 25, 2018

trivikr pushed a commit to trivikr/node that referenced this pull request Mar 26, 2018
Fixes test/parallel/test-console-no-swallow-stack-exceeded.js

PR-URL: nodejs#19423
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@trivikr
Copy link
Member

trivikr commented Mar 26, 2018

Landed in d49661b

@trivikr trivikr closed this Mar 26, 2018
@dbkaplun dbkaplun deleted the console-no-swallow-stack-overflow branch March 26, 2018 03:06
targos pushed a commit that referenced this pull request Mar 27, 2018
Fixes test/parallel/test-console-no-swallow-stack-exceeded.js

PR-URL: #19423
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants