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

cli: add --trace-uncaught flag #30025

Closed
wants to merge 2 commits into from

Conversation

@addaleax
Copy link
Member

addaleax commented Oct 18, 2019

Add a flag that makes Node.js print the stack trace at the
time of throwing uncaught exceptions, rather than at the
creation of the Error object, if there is any.

This is disabled by default because it affects GC behavior.

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
Add a flag that makes Node.js print the stack trace at the
time of *throwing* uncaught exceptions, rather than at the
creation of the `Error` object, if there is any.

This is disabled by default because it affects GC behavior.
@nodejs-github-bot

This comment has been minimized.

@gireeshpunathil

This comment has been minimized.

Copy link
Member

gireeshpunathil commented Oct 18, 2019

@addaleax - can you please throw more light on what behavior change it causes to gc?

@addaleax

This comment has been minimized.

Copy link
Member Author

addaleax commented Oct 18, 2019

@gireeshpunathil As far as I could tell, it makes functions keep alive stack frames which in turn keep objects that were present in those stack frames alive -- V8 doesn't really document this or what other impact it might have.

You can try it for yourself by enabling this flag by default and running the test suite, some tests that use GC will fail.

@gireeshpunathil

This comment has been minimized.

Copy link
Member

gireeshpunathil commented Oct 19, 2019

@addaleax - I ran the gc tests, but did not see any failures (in osx).

Probably this is not a cause of worry for real applications (that do not depend on exact timing and order of objects being collected)?

@addaleax

This comment has been minimized.

Copy link
Member Author

addaleax commented Oct 19, 2019

@gireeshpunathil

$ ./node --expose-gc --trace-uncaught test/parallel/test-zlib-invalid-input-memory.js
Mismatched noop function calls. Expected exactly 1, actual 0.
    at Object.mustCall (/home/sqrt/src/node/test/common/index.js:337:10)
    at Object.<anonymous> (/home/sqrt/src/node/test/parallel/test-zlib-invalid-input-memory.js:11:21)
[...]

I think static functions that hold on to dynamic data that was passed to them qualifies as a memory leak (even if its size is limited), and I wouldn’t feel comfortable turning this flag on by default (at least not without further research).

@nodejs-github-bot

This comment has been minimized.

@danbev

This comment has been minimized.

Copy link
Member

danbev commented Oct 22, 2019

Landed in 31217a8.

@danbev danbev closed this Oct 22, 2019
danbev added a commit that referenced this pull request Oct 22, 2019
Add a flag that makes Node.js print the stack trace at the
time of *throwing* uncaught exceptions, rather than at the
creation of the `Error` object, if there is any.

This is disabled by default because it affects GC behavior.

PR-URL: #30025
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
MylesBorins added a commit that referenced this pull request Oct 23, 2019
Add a flag that makes Node.js print the stack trace at the
time of *throwing* uncaught exceptions, rather than at the
creation of the `Error` object, if there is any.

This is disabled by default because it affects GC behavior.

PR-URL: #30025
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
MylesBorins added a commit that referenced this pull request Oct 23, 2019
Add a flag that makes Node.js print the stack trace at the
time of *throwing* uncaught exceptions, rather than at the
creation of the `Error` object, if there is any.

This is disabled by default because it affects GC behavior.

PR-URL: #30025
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
targos added a commit that referenced this pull request Nov 5, 2019
Notable changes:

* cli:
  * Added a new flag (`--trace-uncaught`) that makes Node.js print the
    stack trace at the time of throwing uncaught exceptions, rather than
    at the creation of the `Error` object, if there is any. This is
    disabled by default because it affects GC behavior.
    #30025
* crypto
  * Added `Hash.prototype.copy()` method. It returns a new `Hash` object
    with its internal state cloned from the original one.
    #29910
* dgram
  * Added source-specific multicast support. This adds methods to
    Datagram sockets to support RFC 4607
    (https://tools.ietf.org/html/rfc4607) for IPv4 and IPv6.
    #15735
* fs
  * Added a `bufferSize` option to `fs.opendir()`. It allows to control
    the number of entries that are buffered internally when reading from
    the directory. #30114
* meta
  * Added Chengzhong Wu (https://github.com/legendecas) to
    collaborators. #30115

PR-URL: #30262
@targos targos referenced this pull request Nov 5, 2019
targos added a commit that referenced this pull request Nov 5, 2019
Notable changes:

* cli:
  * Added a new flag (`--trace-uncaught`) that makes Node.js print the
    stack trace at the time of throwing uncaught exceptions, rather than
    at the creation of the `Error` object, if there is any. This is
    disabled by default because it affects GC behavior.
    #30025
* crypto
  * Added `Hash.prototype.copy()` method. It returns a new `Hash` object
    with its internal state cloned from the original one.
    #29910
* dgram
  * Added source-specific multicast support. This adds methods to
    Datagram sockets to support RFC 4607
    (https://tools.ietf.org/html/rfc4607) for IPv4 and IPv6.
    #15735
* fs
  * Added a `bufferSize` option to `fs.opendir()`. It allows to control
    the number of entries that are buffered internally when reading from
    the directory. #30114
* meta
  * Added Chengzhong Wu (https://github.com/legendecas) to
    collaborators. #30115

PR-URL: #30262
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.