child_process: fix stdio sockets creation #18701
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
`readable` and `writable` properties can be passed directly to the `net.Socket` constructor. This change also avoids an unnecessary call to `read(0)` on the `stdin` socket. This behavior was disclosed when trying to merge `libuv@1.19.0` and specifically this commit: libuv/libuv@fd04939. Refs: libuv/libuv#1655
nodejs-github-bot
added
the
child_process
addaleax
approved these changes
Feb 10, 2018
BridgeAR
approved these changes
Feb 10, 2018
|
@santigimeno please always trigger a CI after creating a PR :-) |
BridgeAR
added
the
author ready
|
@BridgeAR yeah, I had done so but forgot to post it here: https://ci.nodejs.org/job/node-test-pull-request/13083/. Anyway, thanks for the heads up. |
santigimeno
force-pushed
the
fix_socket_creation_child
branch
from
04018b8
to
28e7555
Compare
cjihrig
approved these changes
Feb 11, 2018
vtjnash
added a commit
to vtjnash/libuv
that referenced
this pull request
Feb 13, 2018
Windows is better about already doing this, so this will make the behavior of these flags more consistent across platforms. It also is just better to set these flags to reflect the actual mode of the stream, rather than guessing at it based on typical usage. Refs: libuv#1655 Refs: nodejs/node#18701
|
Landed in b74a6da 🎉 |
BridgeAR
pushed a commit
to BridgeAR/node
that referenced
this pull request
Feb 16, 2018
`readable` and `writable` properties can be passed directly to the `net.Socket` constructor. This change also avoids an unnecessary call to `read(0)` on the `stdin` socket. This behavior was disclosed when trying to merge `libuv@1.19.0` and specifically this commit: libuv/libuv@fd04939. PR-URL: nodejs#18701 Refs: libuv/libuv#1655 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Feb 21, 2018
`readable` and `writable` properties can be passed directly to the `net.Socket` constructor. This change also avoids an unnecessary call to `read(0)` on the `stdin` socket. This behavior was disclosed when trying to merge `libuv@1.19.0` and specifically this commit: libuv/libuv@fd04939. PR-URL: #18701 Refs: libuv/libuv#1655 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Merged
MylesBorins
pushed a commit
that referenced
this pull request
Feb 21, 2018
`readable` and `writable` properties can be passed directly to the `net.Socket` constructor. This change also avoids an unnecessary call to `read(0)` on the `stdin` socket. This behavior was disclosed when trying to merge `libuv@1.19.0` and specifically this commit: libuv/libuv@fd04939. PR-URL: #18701 Refs: libuv/libuv#1655 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
santigimeno
pushed a commit
to libuv/libuv
that referenced
this pull request
Feb 25, 2018
`UV_READABLE_PIPE` and `UV_WRITABLE_PIPE` flags should be honored on unices. Windows is better about already doing this, so this will make the behavior of these flags more consistent across platforms. It also is just better to set these flags to reflect the actual mode of the stream, rather than guessing at it based on typical usage. Refs: #1655 Refs: nodejs/node#18701 PR-URL: #1741 Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MayaLekova
pushed a commit
to MayaLekova/node
that referenced
this pull request
May 8, 2018
`readable` and `writable` properties can be passed directly to the `net.Socket` constructor. This change also avoids an unnecessary call to `read(0)` on the `stdin` socket. This behavior was disclosed when trying to merge `libuv@1.19.0` and specifically this commit: libuv/libuv@fd04939. PR-URL: nodejs#18701 Refs: libuv/libuv#1655 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins
pushed a commit
that referenced
this pull request
Aug 7, 2018
`readable` and `writable` properties can be passed directly to the `net.Socket` constructor. This change also avoids an unnecessary call to `read(0)` on the `stdin` socket. This behavior was disclosed when trying to merge `libuv@1.19.0` and specifically this commit: libuv/libuv@fd04939. PR-URL: #18701 Refs: libuv/libuv#1655 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
rvagg
pushed a commit
that referenced
this pull request
Aug 16, 2018
`readable` and `writable` properties can be passed directly to the `net.Socket` constructor. This change also avoids an unnecessary call to `read(0)` on the `stdin` socket. This behavior was disclosed when trying to merge `libuv@1.19.0` and specifically this commit: libuv/libuv@fd04939. PR-URL: #18701 Refs: libuv/libuv#1655 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Merged
jbunton-atlassian
added a commit
to jbunton-atlassian/node
that referenced
this pull request
Dec 19, 2018
Since commit 1d90700 child processes have not correctly emitted a close event for stdin. This change triggers a zero-byte read from the Socket constructor for any child process sockets. That allows the close event to be detected. Refs: nodejs#18701
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
readableandwritableproperties can be passed directly to thenet.Socketconstructor. This change also avoids an unnecessary callto
read(0)on thestdinsocket. This behavior was disclosed whentrying to merge
libuv@1.19.0and specifically this commit:libuv/libuv@fd04939.
Refs: libuv/libuv#1655
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
child_process