Skip to content

Conversation

@cjihrig
Copy link
Contributor

This flag allows heap snapshots to be captured without modifying application code. IMO, this is a big part of the "heapdump in core" use case.

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

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-botnodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Apr 8, 2019
Copy link
Member

@jasnelljasnell left a comment

Choose a reason for hiding this comment

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

LGTM with @joyeecheung's comment addressed

@richardlau
Copy link
Member

Does this work on Windows? (I'm guessing not, at least based on the equivalent test in

if(!common.isWindows){
// Verify that process.report.signal behaves properly.
assert.strictEqual(process.report.signal,'SIGUSR2');
common.expectsError(()=>{
process.report.signal={};
},{code: 'ERR_INVALID_ARG_TYPE'});
common.expectsError(()=>{
process.report.signal='foo';
},{code: 'ERR_UNKNOWN_SIGNAL'});
assert.strictEqual(process.report.signal,'SIGUSR2');
process.report.signal='SIGUSR1';
assert.strictEqual(process.report.signal,'SIGUSR1');
// Verify that the interaction between reportOnSignal and signal is correct.
process.report.signal='SIGUSR2';
process.report.reportOnSignal=false;
assert.strictEqual(process.listenerCount('SIGUSR2'),0);
process.report.reportOnSignal=true;
assert.strictEqual(process.listenerCount('SIGUSR2'),1);
process.report.signal='SIGUSR1';
assert.strictEqual(process.listenerCount('SIGUSR2'),0);
assert.strictEqual(process.listenerCount('SIGUSR1'),1);
process.report.reportOnSignal=false;
assert.strictEqual(process.listenerCount('SIGUSR1'),0);
}
).

@cjihrigcjihrig added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 11, 2019
@cjihrigcjihrigforce-pushed the heap-signal branch 2 times, most recently from e89e9d6 to 2520bf7CompareApril 11, 2019 01:25
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 12, 2019

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

EDIT(cjihrig): CI was green.

This flag allows heap snapshots to be captured without modifying application code. PR-URL: nodejs#27133 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@cjihrig@nodejs-github-bot@richardlau@sam-github@bnoordhuis@jasnell@addaleax@joyeecheung@BethGriggs