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

child_process: fix data loss with readable event (v0.12) #5037

Closed

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Feb 2, 2016

This commit prevents child process stdio streams from being automatically flushed on child process exit/close if a 'readable' event handler has been attached at the time of exit.

Without this, child process stdio data can be lost if the process exits quickly and a read() (e.g. from a 'readable' handler) hasn't had the chance to get called yet.

Fixes: #5034

@mscdex mscdex added child_process Issues and PRs related to the child_process subsystem. land-on-v0.12 labels Feb 2, 2016
@jasnell
Copy link
Member

jasnell commented Feb 2, 2016

LGTM

This commit prevents child process stdio streams from being
automatically flushed on child process exit/close if a 'readable'
event handler has been attached at the time of exit.

Without this, child process stdio data can be lost if the process
exits quickly and a `read()` (e.g. from a 'readable' handler)
hasn't had the chance to get called yet.

Fixes: nodejs#5034
@mscdex mscdex force-pushed the fix-child-process-flush-stdio-v012 branch from 9e9e99f to 3e7af8d Compare February 4, 2016 02:23
@mscdex
Copy link
Contributor Author

mscdex commented Feb 4, 2016

mscdex added a commit that referenced this pull request Feb 11, 2016
This commit prevents child process stdio streams from being
automatically flushed on child process exit/close if a 'readable'
event handler has been attached at the time of exit.

Without this, child process stdio data can be lost if the process
exits quickly and a `read()` (e.g. from a 'readable' handler)
hasn't had the chance to get called yet.

Fixes: #5034
PR-URL: #5037
Reviewed-By: James M Snell <jasnell@gmail.com>
@mscdex
Copy link
Contributor Author

mscdex commented Feb 11, 2016

Landed in 18ab5d4.

@mscdex mscdex closed this Feb 11, 2016
@mscdex mscdex deleted the fix-child-process-flush-stdio-v012 branch February 11, 2016 17:51
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
This commit prevents child process stdio streams from being
automatically flushed on child process exit/close if a 'readable'
event handler has been attached at the time of exit.

Without this, child process stdio data can be lost if the process
exits quickly and a `read()` (e.g. from a 'readable' handler)
hasn't had the chance to get called yet.

Fixes: #5034
PR-URL: #5037
Reviewed-By: James M Snell <jasnell@gmail.com>
jBarz pushed a commit to ibmruntimes/node that referenced this pull request Nov 4, 2016
This commit prevents child process stdio streams from being
automatically flushed on child process exit/close if a 'readable'
event handler has been attached at the time of exit.

Without this, child process stdio data can be lost if the process
exits quickly and a `read()` (e.g. from a 'readable' handler)
hasn't had the chance to get called yet.

Fixes: nodejs/node#5034
PR-URL: nodejs/node#5037
Reviewed-By: James M Snell <jasnell@gmail.com>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants