Skip to content

Conversation

thlorenz
Copy link
Contributor

Affected core subsystem

child_process

Description of change

  • initializing _unref to false when child process is created
  • updating that value each time either unref() or ref() is invoked
  • this mirrors the implementation in lib/net.js
  • adding test to check unrefdness tracking

R= @Fishrock123
R= @trevnorris

- initializing `_unref` to `false` when child process is created
- updating that value each time either `unref()` or `ref()` is invoked
- this mirrors the implementation in `lib/net.js`
- adding test to check unrefdness tracking
@mscdex mscdex added the child_process Issues and PRs related to the child_process subsystem. label Mar 21, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Mar 21, 2016

LGTM, but the tests would be a little more readable, IMO, if you used assert.strictEqual() and compared against a Boolean instead of assert.ok().

@jasnell
Copy link
Member

jasnell commented Mar 21, 2016

hmm.. how would this be used? If it's intended to be used by userland code, then the _ prefix is inappropriate.

@jasnell jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Mar 21, 2016
@Fishrock123
Copy link
Contributor

Been thinking about this sort of thing for a while, I think it would be best for every handle to have this sort of property in c++, accessible to JS, if possible.

@thlorenz
Copy link
Contributor Author

@jasnell see this comment in the related PR. We probably should decide on one of those for a discussion thread since they are so similar.

Fishrock123 added a commit to Fishrock123/node that referenced this pull request Apr 7, 2016
This allows third-party tools to check whether or not a handle that
can be unreferenced is unreferenced at a particular time.
Notably, this should be helpful for inspection via AsyncWrap.

Also, this is useful even to node's internals, particularly timers.

Refs: nodejs#5828
Refs: nodejs#5827
PR-URL: nodejs#5834
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
@cjihrig
Copy link
Contributor

cjihrig commented Apr 20, 2016

Closing this for the same reasons as #5828.

@cjihrig cjihrig closed this Apr 20, 2016
evanlucas pushed a commit that referenced this pull request May 17, 2016
This allows third-party tools to check whether or not a handle that
can be unreferenced is unreferenced at a particular time.
Notably, this should be helpful for inspection via AsyncWrap.

Also, this is useful even to node's internals, particularly timers.

Refs: #5828
Refs: #5827
PR-URL: #5834
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. semver-minor PRs 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.

5 participants