-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
nodejs sometimes leaves stdout/stderr in non-blocking mode #14752
Comments
@chriskuehl Does this still reproduce on v9.2.0? |
@maclover7 yes it does, same reproduction steps. Here's output based on the test repro I linked above:
|
I ran into this today as well and I have a reproducer without docker: https://gist.github.com/ivan/e1eafebc7e5273b6f80d2cc9bdb96a38 (tested on a recent Linux 4.4 and 4.14, nodejs 8.9.2 and 9.2.0) |
Record the state of the stdio file descriptors on start-up and restore them to that state on exit. This should prevent issues where node.js sometimes leaves stdio in raw or non-blocking mode. Fixes: nodejs#14752
Potential fix in #17737. If anyone here can confirm it fixes the issue for them? By the way, a python2 version of the test case: import fcntl, os, sys, subprocess
for _ in range(2):
nb = fcntl.fcntl(1, fcntl.F_GETFL) & os.O_NONBLOCK
print("stdout blocking? {}".format(nb == 0))
nb = fcntl.fcntl(2, fcntl.F_GETFL) & os.O_NONBLOCK
print("stderr blocking? {}".format(nb == 0))
sys.stdout.flush()
subprocess.check_output(['out/Release/node', '-e', '2+2']) |
I did not test the fix, but I wonder if it solves the problem when the parent process doesn't wait for the node process to exit? (Is there something that prevents stdout from going nonblocking in this case?) |
Depends on what you mean. Whether there's someone waiting doesn't matter to node.js but if another process concurrently tries to read or write to the same file description (note: not file descriptor), it might observe non-blocking I/O. That's hard to remedy in the UNIX model, though - you can't always reopen stdio to get fresh file descriptions. |
potential fix in #20592 . @chriskuehl are you in a position to apply the patch , validate and confirm? or you want to wait for a binary? |
Record the state of the stdio file descriptors on start-up and restore them to that state on exit. This should prevent issues where node.js sometimes leaves stdio in raw or non-blocking mode. Co-authored-by: Krzysztof Taborski <taborskikrzysztof@gmail.com> PR-URL: #20592 Fixes: #14752 Fixes: #21020 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
I guess we have to reopen this :( (For context, the fix was reverted in #21257) |
Node leaves stdio file descriptors in non-blocking mode when exiting. This has been reported, fixed, unfixed, ad nauseum. nodejs/node#14752 nodejs/node#17737 nodejs/node#20592 nodejs/node#21257 Should this become a problem in more places, we could add such a workaround elsewhere. But for now I'm limiting the ugliness to the unit-tests container, where we see this cause a lot of failures. Closes cockpit-project#9484
Node leaves stdio file descriptors in non-blocking mode when exiting. This has been reported, fixed, unfixed, ad nauseum. nodejs/node#14752 nodejs/node#17737 nodejs/node#20592 nodejs/node#21257 Should this become a problem in more places, we could add such a workaround elsewhere. But for now I'm limiting the ugliness to the unit-tests container, where we see this cause a lot of failures. Closes #9484
Node leaves stdio file descriptors in non-blocking mode when exiting. This has been reported, fixed, unfixed, ad nauseum. nodejs/node#14752 nodejs/node#17737 nodejs/node#20592 nodejs/node#21257 Stolen from cockpit-project/cockpit@ef50d97cf4
The build is failing with errors writing to stdout due to a combination of issues: * ./mach spawns make and reads the output one byte at a time * There are lots of make output due to the rustc debug logs enabled * node-js is run as part of the builds, and it leaves stdout non-blocking: nodejs/node#14752 So, eventually some other commmand can't handle stdout being non-blocking, but the pipe becomes (temporarily) full as mach can't keep up, and the whole shitshow comes to a stop.
Record the state of the stdio file descriptors on start-up and restore them to that state on exit. This should prevent issues where node.js sometimes leaves stdio in raw or non-blocking mode. This is a reworked version of commit c2c9c0c from May 2018 that was reverted in commit 14dc17d from June 2018. The revert was a little light on details but I infer that the problem was caused by a missing call to `uv_tty_reset_mode()`. Apropos the NOLINT comments: cpplint doesn't understand do/while statements, it thinks they're while statements without a body. Fixes: nodejs#14752 Fixes: nodejs#21020 Original-PR-URL: nodejs#20592
Record the state of the stdio file descriptors on start-up and restore them to that state on exit. This should prevent issues where node.js sometimes leaves stdio in raw or non-blocking mode. This is a reworked version of commit c2c9c0c from May 2018 that was reverted in commit 14dc17d from June 2018. The revert was a little light on details but I infer that the problem was caused by a missing call to `uv_tty_reset_mode()`. Apropos the NOLINT comments: cpplint doesn't understand do/while statements, it thinks they're while statements without a body. Fixes: #14752 Fixes: #21020 Original-PR-URL: #20592 PR-URL: #24260 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann (רפאל פלחי) <refack@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 --HG-- extra : moz-landing-system : lando
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 --HG-- extra : moz-landing-system : lando
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 UltraBlame original commit: 562655fedb9df034ae1f8c32cd730f6498d501a7
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 UltraBlame original commit: 562655fedb9df034ae1f8c32cd730f6498d501a7
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 UltraBlame original commit: 562655fedb9df034ae1f8c32cd730f6498d501a7
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 --HG-- extra : moz-landing-system : lando
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 --HG-- extra : moz-landing-system : lando
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 --HG-- extra : source : 562655fedb9df034ae1f8c32cd730f6498d501a7
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986 --HG-- extra : moz-landing-system : lando
This works around nodejs/node#14752, which causes problems with make. Differential Revision: https://phabricator.services.mozilla.com/D35986
Quick question: Given that the merged fix uses In that case programs that call node as a subprocess must still take extra precautions like Firefox does here by never giving |
@nh2 Yes, if node is hard-killed then it won't have a chance to reset stdio. That caveat applies to SIGKILL (because it can't be caught) but not e.g. SIGINT and SIGTERM, their signal handlers restore stdio. (SIGSEGV and SIGBUS currently don't but that could be added. Then again, those signals mean the program is in an undefined state and trying to clean up might make it worse.)
You can, just reset their file descriptors to blocking I/O and cooked mode afterwards. |
Linux 19083a5259e3 4.4.0-1022-aws #31-Ubuntu SMP Tue Jun 27 11:27:55 UTC 2017 x86_64 GNU/Linux
node appears to leave stdout/stderr in non-blocking mode in certain cases, which can cause errors in other programs which assume these are in blocking mode. I'm not certain if this is a bug or not, but am just trying to understand this behavior better.
I'm having trouble finding authoritative information about non-blocking mode and whether it's "correct" for programs to assume that they'll be given stdout/stderr in blocking mode, but it does appear that a lot of programs aren't able to handle having stdout/stderr in blocking mode (Python being a notable case). It's also strange that node only appears to do this in specific cases (e.g. in Docker without a TTY).
Reproduction
I haven't yet been able to find a way to reproduce this without Docker. I've created a simple test repo that prints the status of the blocking flag before and after running nodejs at https://github.com/chriskuehl/nodejs-nonblocking-repro. It shows that the blocking flag on stdout/stderr changes after nodejs is run.
Alternatively, a reproduction using Docker looks like this:
docker run -i node:latest bash
to enter a shell in the container (don't use-t
; it doesn't work if using a tty)python -c 'print "x" * 100000'
inside the container to show that Python can print a lot of text without failingnodejs -e 'console.log(1+1)'
inside the container. After running this command, stdout/stderr have been changed to non-blocking mode.python -c 'print "x" * 100000'
inside the container, which should crash after printing some text (you should see a traceback somewhere, it may be at the top or interlaced in the middle of the printed text)In Python 2, the traceback error is
IOError: [Errno 11] Resource temporarily unavailable
. In Python 3, it's a little more helpful:BlockingIOError: [Errno 11] write could not complete without blocking
.The text was updated successfully, but these errors were encountered: