This repository has been archived by the owner on Apr 22, 2023. It is now read-only.
Close child pipes when child exits (+ test) #3031
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
March 18, 2012 13:12
Windows will not close the pipes until grandchildren exit (even if child exited). Added a test to verify this.
|
Just saw this: #2944. Looks great. |
srl295
pushed a commit
to srl295/node-v0.x-archive
that referenced
this pull request
Sep 25, 2015
Backport c281c15d6dab8370a7805f0717502d260e0ad433 from V8's upstream to allow post-mortem debugging tools to inspect Buffer instances' length. Original commit message: Add JSTypedArray's length in post-mortem metadata. BUG= R=bmeurer@chromium.org Review URL: https://codereview.chromium.org/1363683002 Cr-Commit-Position: refs/heads/master@{#30873} PR: nodejs#3031 PR-URL: nodejs/node#3031 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
richardlau
pushed a commit
to ibmruntimes/node
that referenced
this pull request
Oct 6, 2015
Notable changes
* http:
- Fix out-of-order 'finish' event bug in pipelining that can abort
execution, fixes DoS vulnerability CVE-2015-7384
(Fedor Indutny) nodejs#3128
- Account for pending response data instead of just the data on the
current request to decide whether pause the socket or not
(Fedor Indutny) nodejs#3128
* libuv: Upgraded from v1.7.4 to v1.7.5, see release notes for details
(Saúl Ibarra Corretgé) nodejs#3010
- A better rwlock implementation for all Windows versions
- Improved AIX support
* v8:
- Upgraded from v4.5.103.33 to v4.5.103.35 (Ali Ijaz Sheikh) nodejs#3117
- Backported f782159 from v8's upstream to help speed up Promise
introspection (Ben Noordhuis) nodejs#3130
- Backported c281c15 from v8's upstream to add JSTypedArray length
in post-mortem metadata (Julien Gilli) nodejs#3031
PR-URL: nodejs/node#3128
richardlau
pushed a commit
to ibmruntimes/node
that referenced
this pull request
Oct 6, 2015
Backport c281c15d6dab8370a7805f0717502d260e0ad433 from V8's upstream to allow post-mortem debugging tools to inspect Buffer instances' length. Original commit message: Add JSTypedArray's length in post-mortem metadata. BUG= R=bmeurer@chromium.org Review URL: https://codereview.chromium.org/1363683002 Cr-Commit-Position: refs/heads/master@{#30873} PR: nodejs#3031 PR-URL: nodejs/node#3031 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
richardlau
pushed a commit
to ibmruntimes/node
that referenced
this pull request
Oct 6, 2015
Notable changes
* http:
- Fix out-of-order 'finish' event bug in pipelining that can abort
execution, fixes DoS vulnerability CVE-2015-7384
(Fedor Indutny) nodejs#3128
- Account for pending response data instead of just the data on the
current request to decide whether pause the socket or not
(Fedor Indutny) nodejs#3128
* libuv: Upgraded from v1.7.4 to v1.7.5, see release notes for details
(Saúl Ibarra Corretgé) nodejs#3010
- A better rwlock implementation for all Windows versions
- Improved AIX support
* v8:
- Upgraded from v4.5.103.33 to v4.5.103.35 (Ali Ijaz Sheikh) nodejs#3117
- Backported f782159 from v8's upstream to help speed up Promise
introspection (Ben Noordhuis) nodejs#3130
- Backported c281c15 from v8's upstream to add JSTypedArray length
in post-mortem metadata (Julien Gilli) nodejs#3031
PR-URL: nodejs/node#3128
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This essentially fixes #818: when a child process exits,
stdoutandstderrare now closed.There seems to be a difference in behavior between Windows and Unix in this scenario. Even if a child process spawns a grandchild, on unix, when a child exits, the stdout/stderr pipes associated with it are broken so that parent process streams are closed automatically. On Windows, the pipes remain open until the grandchild exits.
The current implementation in node will wait for the streams to be closed before emitting the
exitevent (even though the child already exited).See discussion in joyent/libuv#343. It seems that the preferred solution is to make this fix at the node level instead of libuv.
The associated test (
simple/test-child-process-tree) fails without this patch only on Windows.Contributed: @yosefd @saary