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

node exits with error: getWindowSize EINTR #5737

Closed
jkryl opened this issue Mar 16, 2016 · 2 comments · Fixed by #5994
Closed

node exits with error: getWindowSize EINTR #5737

jkryl opened this issue Mar 16, 2016 · 2 comments · Fixed by #5994
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding. smartos Issues and PRs related to the SmartOS platform.

Comments

@jkryl
Copy link

jkryl commented Mar 16, 2016

  • Version: v4.3.0
  • Platform: SunOS with 32bit userland

Occasionally (happens once a month or so) the node process exits with following message on screen:

Error: getWindowSize EINTR
    at exports._errnoException (util.js:870:11)
    at WriteStream._refreshSize (tty.js:82:24)
    at process.<anonymous> (node.js:638:18)
    at emitNone (events.js:72:20)
    at process.emit (events.js:166:7)
    at Signal.wrap.onsignal (node.js:802:46)

What I think that happens is that the node is trying to get new size of screen after receiving SIGWINCH signal and the call to getWindowSize is interrupted by another signal and because SA_RESTART flag for sigaction() is not set by default on solaris unlike on other platforms, the interrupted syscall is not repeated and EINTR error is emitted on stream (

this.emit('error', errnoException(err, 'getWindowSize'));
) and makes the program to exit. I have no reproducible test case or proof that my assumptions are correct. Also I have seen a couple of places in nodejs code where EINTR is explicitly checked and function call repeated if that's the case. Perhaps something similar could be done for this piece of code as well.

@bnoordhuis bnoordhuis added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Mar 16, 2016
@bnoordhuis
Copy link
Member

That sounds plausible. It's probably Solaris-specific because I don't think the other Unixes return EINTR from ioctl(TIOCGWINSZ).

This patch should help. I'll get it upstreamed in libuv:

diff --git a/deps/uv/src/unix/tty.c b/deps/uv/src/unix/tty.c
index 7cc5b71..f62b59f 100644
--- a/deps/uv/src/unix/tty.c
+++ b/deps/uv/src/unix/tty.c
@@ -185,8 +185,13 @@ int uv_tty_set_mode(uv_tty_t* tty, uv_tty_mode_t mode) {

 int uv_tty_get_winsize(uv_tty_t* tty, int* width, int* height) {
   struct winsize ws;
+  int err;
+
+  do
+    err = ioctl(uv__stream_fd(tty), TIOCGWINSZ, &ws);
+  while (err == -1 && errno == EINTR);

-  if (ioctl(uv__stream_fd(tty), TIOCGWINSZ, &ws))
+  if (err)
     return -errno;

   *width = ws.ws_col;

@Fishrock123 Fishrock123 added the smartos Issues and PRs related to the SmartOS platform. label Mar 16, 2016
@jkryl
Copy link
Author

jkryl commented Mar 16, 2016

Thanks for quick response and the patch!

bnoordhuis added a commit to bnoordhuis/libuv that referenced this issue Mar 20, 2016
Some platforms (notably Solaris) can fail in this ioctl() if interrupted
by a signal.  Retry the system call when that happens.

Fixes: nodejs/node#5737
bnoordhuis added a commit to bnoordhuis/libuv that referenced this issue Mar 20, 2016
Some platforms (notably Solaris) can fail in this ioctl() if interrupted
by a signal.  Retry the system call when that happens.

Fixes: nodejs/node#5737
bnoordhuis added a commit to bnoordhuis/libuv that referenced this issue Mar 20, 2016
Some platforms (notably Solaris) can fail in this ioctl() if interrupted
by a signal.  Retry the system call when that happens.

Fixes: nodejs/node#5737
PR-URL: libuv#772
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
saghul added a commit to saghul/node that referenced this issue Apr 7, 2016
saghul added a commit that referenced this issue Apr 7, 2016
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this issue Apr 19, 2016
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
saghul added a commit to saghul/node that referenced this issue Jul 11, 2016
Fixes: nodejs#5737
Fixes: nodejs#4643
Fixes: nodejs#4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: nodejs#3594
PR-URL: nodejs#5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this issue Jul 11, 2016
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this issue Jul 11, 2016
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this issue Jul 12, 2016
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this issue Jul 14, 2016
Fixes: #5737
Fixes: #4643
Fixes: #4291
Fixes: nodejs/node-v0.x-archive#8960
Refs: #3594
PR-URL: #5994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libuv Issues and PRs related to the libuv dependency or the uv binding. smartos Issues and PRs related to the SmartOS platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants