From 105e4dcb231f88c8e9a71cca85be9af9267022f4 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 22 Apr 2013 07:46:21 +0200 Subject: [PATCH 1/9] unix: silence STATIC_ASSERT compiler warnings Fix the following two warnings: src/unix/core.c:74:1: warning: ISO C90 forbids array 'static_assert_failed' whose size can't be evaluated [-Wvla] src/unix/core.c:76:1: warning: ISO C90 forbids array 'static_assert_failed' whose size can't be evaluated [-Wvla] --- src/unix/core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/unix/core.c b/src/unix/core.c index b8bae48a5..7ab9bd6c7 100644 --- a/src/unix/core.c +++ b/src/unix/core.c @@ -71,10 +71,8 @@ STATIC_ASSERT(sizeof(&((uv_buf_t*) 0)->base) == sizeof(((struct iovec*) 0)->iov_base)); STATIC_ASSERT(sizeof(&((uv_buf_t*) 0)->len) == sizeof(((struct iovec*) 0)->iov_len)); -STATIC_ASSERT((uintptr_t) &((uv_buf_t*) 0)->base == - (uintptr_t) &((struct iovec*) 0)->iov_base); -STATIC_ASSERT((uintptr_t) &((uv_buf_t*) 0)->len == - (uintptr_t) &((struct iovec*) 0)->iov_len); +STATIC_ASSERT(offsetof(uv_buf_t, base) == offsetof(struct iovec, iov_base)); +STATIC_ASSERT(offsetof(uv_buf_t, len) == offsetof(struct iovec, iov_len)); uint64_t uv_hrtime(void) { From cd10637d0b3c03e28831eca2d66d8bda3216b10c Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 22 Apr 2013 08:24:57 +0200 Subject: [PATCH 2/9] linux: don't use fopen() in uv_resident_set_memory() RSS is a reflection of the number of pages that a process has mapped. glibc implements fopen() in terms of mmap() which means that trying to read the number of mapped pages changes it. Switch to open(). --- src/unix/linux-core.c | 129 +++++++++++++++--------------------------- 1 file changed, 45 insertions(+), 84 deletions(-) diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c index daab6f196..35374b17d 100644 --- a/src/unix/linux-core.c +++ b/src/unix/linux-core.c @@ -284,98 +284,59 @@ uint64_t uv_get_total_memory(void) { uv_err_t uv_resident_set_memory(size_t* rss) { - FILE* f; - int itmp; - char ctmp; - unsigned int utmp; - size_t page_size = getpagesize(); - char *cbuf; - int foundExeEnd; - char buf[PATH_MAX + 1]; + char buf[1024]; + const char* s; + ssize_t n; + long val; + int fd; + int i; - f = fopen("/proc/self/stat", "r"); - if (!f) return uv__new_sys_error(errno); + do + fd = open("/proc/self/stat", O_RDONLY); + while (fd == -1 && errno == EINTR); - /* PID */ - if (fscanf(f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* Exec file */ - cbuf = buf; - foundExeEnd = 0; - if (fscanf (f, "%c", cbuf++) == 0) goto error; - while (1) { - if (fscanf(f, "%c", cbuf) == 0) goto error; - if (*cbuf == ')') { - foundExeEnd = 1; - } else if (foundExeEnd && *cbuf == ' ') { - *cbuf = 0; - break; - } + if (fd == -1) + return uv__new_sys_error(errno); - cbuf++; + do + n = read(fd, buf, sizeof(buf) - 1); + while (n == -1 && errno == EINTR); + + SAVE_ERRNO(close(fd)); + if (n == -1) + return uv__new_sys_error(errno); + buf[n] = '\0'; + + s = strchr(buf, ' '); + if (s == NULL) + goto err; + + s += 1; + if (*s != '(') + goto err; + + s = strchr(s, ')'); + if (s == NULL) + goto err; + + for (i = 1; i <= 22; i++) { + s = strchr(s + 1, ' '); + if (s == NULL) + goto err; } - /* State */ - if (fscanf (f, "%c ", &ctmp) == 0) goto error; /* coverity[secure_coding] */ - /* Parent process */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* Process group */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* Session id */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* TTY */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* TTY owner process group */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* Flags */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* Minor faults (no memory page) */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* Minor faults, children */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* Major faults (memory page faults) */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* Major faults, children */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* utime */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* stime */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* utime, children */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* stime, children */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* jiffies remaining in current time slice */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* 'nice' value */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* jiffies until next timeout */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* jiffies until next SIGALRM */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* start time (jiffies since system boot) */ - if (fscanf (f, "%d ", &itmp) == 0) goto error; /* coverity[secure_coding] */ - /* Virtual memory size */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ + errno = 0; + val = strtol(s, NULL, 10); + if (errno != 0) + goto err; + if (val < 0) + goto err; - /* Resident set size */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - *rss = (size_t) utmp * page_size; - - /* rlim */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* Start of text */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* End of text */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - /* Start of stack */ - if (fscanf (f, "%u ", &utmp) == 0) goto error; /* coverity[secure_coding] */ - - fclose (f); + *rss = val * getpagesize(); return uv_ok_; -error: - fclose (f); - return uv__new_sys_error(errno); +err: + return uv__new_artificial_error(UV_EINVAL); } From 6595a7732c52eb4f8e57c88655f72997a8567a67 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 22 Apr 2013 17:37:22 -0700 Subject: [PATCH 3/9] 2013.04.24, Version 0.10.5 (Stable) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes since version 0.10.4: * unix: silence STATIC_ASSERT compiler warnings (Ben Noordhuis) * windows: make timers handle large timeouts (Miroslav Bajtoš) * windows: remove superfluous assert statement (Bert Belder) * unix: silence STATIC_ASSERT compiler warnings (Ben Noordhuis) * linux: don't use fopen() in uv_resident_set_memory() (Ben Noordhuis) --- AUTHORS | 1 + ChangeLog | 15 +++++++++++++++ src/version.c | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 70ee0ea34..bfffbbbac 100644 --- a/AUTHORS +++ b/AUTHORS @@ -81,3 +81,4 @@ Marc Schlaich Brian Mazza Nils Maier Nicholas Vavilov +Miroslav Bajtoš diff --git a/ChangeLog b/ChangeLog index 12da5e13b..257d05c04 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2013.04.24, Version 0.10.5 (Stable) + +Changes since version 0.10.4: + +* unix: silence STATIC_ASSERT compiler warnings (Ben Noordhuis) + +* windows: make timers handle large timeouts (Miroslav Bajtoš) + +* windows: remove superfluous assert statement (Bert Belder) + +* unix: silence STATIC_ASSERT compiler warnings (Ben Noordhuis) + +* linux: don't use fopen() in uv_resident_set_memory() (Ben Noordhuis) + + 2013.04.12, Version 0.10.4 (Stable), 85827e26403ac6dfa331af8ec9916ea7e27bd833 Changes since version 0.10.3: diff --git a/src/version.c b/src/version.c index ed2c07ab8..3291d47ee 100644 --- a/src/version.c +++ b/src/version.c @@ -33,7 +33,7 @@ #define UV_VERSION_MAJOR 0 #define UV_VERSION_MINOR 10 #define UV_VERSION_PATCH 5 -#define UV_VERSION_IS_RELEASE 0 +#define UV_VERSION_IS_RELEASE 1 #define UV_VERSION ((UV_VERSION_MAJOR << 16) | \ From a0cb926e2eb9979378f4b71854e8c035035cc3e8 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 22 Apr 2013 17:37:25 -0700 Subject: [PATCH 4/9] Now working on v0.10.6 --- ChangeLog | 2 +- src/version.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 257d05c04..162d4682b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,4 @@ -2013.04.24, Version 0.10.5 (Stable) +2013.04.24, Version 0.10.5 (Stable), 6595a7732c52eb4f8e57c88655f72997a8567a67 Changes since version 0.10.4: diff --git a/src/version.c b/src/version.c index 3291d47ee..6ee0cc934 100644 --- a/src/version.c +++ b/src/version.c @@ -32,8 +32,8 @@ #define UV_VERSION_MAJOR 0 #define UV_VERSION_MINOR 10 -#define UV_VERSION_PATCH 5 -#define UV_VERSION_IS_RELEASE 1 +#define UV_VERSION_PATCH 6 +#define UV_VERSION_IS_RELEASE 0 #define UV_VERSION ((UV_VERSION_MAJOR << 16) | \ From 2400716d875a6481c13fd0b46068ab3b6afa5345 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 23 Apr 2013 22:34:43 -0400 Subject: [PATCH 5/9] stream: fix osx select hack After latest twiddling, `stream->io_watcher.fd` doesn't contain a real stream's file descriptior anymore. The one from `select_state` should be used instead. Zero-initialize `select_state->event` field. --- src/unix/stream.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/unix/stream.c b/src/unix/stream.c index bc9d4f1bb..c0a7dddeb 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -139,7 +139,7 @@ static void uv__stream_osx_select(void* arg) { stream = arg; s = stream->select; - fd = stream->io_watcher.fd; + fd = s->fd; if (fd > s->int_fd) max_fd = fd; @@ -305,6 +305,7 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) { if (s == NULL) return uv__set_artificial_error(stream->loop, UV_ENOMEM); + s->events = 0; s->fd = *fd; if (uv_async_init(stream->loop, &s->async, uv__stream_osx_select_cb)) { From ac4e7e7ff2d947297ac2995561e49c0e3efdafd9 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 26 Apr 2013 23:43:28 +0400 Subject: [PATCH 6/9] stream: fix small nit in select hack, add test --- build.mk | 1 + src/unix/stream.c | 10 +++--- test/test-list.h | 7 ++++ test/test-osx-select.c | 82 ++++++++++++++++++++++++++++++++++++++++++ uv.gyp | 1 + 5 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 test/test-osx-select.c diff --git a/build.mk b/build.mk index 1986e7824..61a9f4af8 100644 --- a/build.mk +++ b/build.mk @@ -88,6 +88,7 @@ TESTS= \ test/test-loop-stop.o \ test/test-multiple-listen.o \ test/test-mutexes.o \ + test/test-osx-select.o \ test/test-pass-always.o \ test/test-ping-pong.o \ test/test-pipe-bind-error.o \ diff --git a/src/unix/stream.c b/src/unix/stream.c index c0a7dddeb..485328df2 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -202,12 +202,14 @@ static void uv__stream_osx_select(void* arg) { if (FD_ISSET(fd, &swrite)) events |= UV__POLLOUT; - uv_mutex_lock(&s->mutex); - s->events |= events; - uv_mutex_unlock(&s->mutex); + assert(events != 0 || FD_ISSET(s->int_fd, &sread)); + if (events != 0) { + uv_mutex_lock(&s->mutex); + s->events |= events; - if (events != 0) uv_async_send(&s->async); + uv_mutex_unlock(&s->mutex); + } } } diff --git a/test/test-list.h b/test/test-list.h index 016f62711..4c0158589 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -220,6 +220,9 @@ TEST_DECLARE (we_get_signal) TEST_DECLARE (we_get_signals) TEST_DECLARE (signal_multiple_loops) #endif +#ifdef __APPLE__ +TEST_DECLARE (osx_select) +#endif HELPER_DECLARE (tcp4_echo_server) HELPER_DECLARE (tcp6_echo_server) HELPER_DECLARE (udp4_echo_server) @@ -440,6 +443,10 @@ TASK_LIST_START TEST_ENTRY (signal_multiple_loops) #endif +#ifdef __APPLE__ + TEST_ENTRY (osx_select) +#endif + TEST_ENTRY (fs_file_noent) TEST_ENTRY (fs_file_nametoolong) TEST_ENTRY (fs_file_loop) diff --git a/test/test-osx-select.c b/test/test-osx-select.c new file mode 100644 index 000000000..5c2cbd4d9 --- /dev/null +++ b/test/test-osx-select.c @@ -0,0 +1,82 @@ +/* Copyright Joyent, Inc. and other Node contributors. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include "uv.h" +#include "task.h" + +#ifdef __APPLE__ + +#include +#include + +static int read_count; + + +static uv_buf_t alloc_cb(uv_handle_t* handle, size_t size) { + static char buf[1024]; + + return uv_buf_init(buf, ARRAY_SIZE(buf)); +} + + +static void read_cb(uv_stream_t* stream, ssize_t nread, uv_buf_t buf) { + fprintf(stdout, "got data %d\n", ++read_count); + + if (read_count == 3) + uv_close((uv_handle_t*) stream, NULL); +} + + +TEST_IMPL(osx_select) { + int r; + int fd; + size_t i; + size_t len; + const char* str; + uv_tty_t tty; + + fd = open("/dev/tty", O_RDONLY); + + ASSERT(fd >= 0); + + r = uv_tty_init(uv_default_loop(), &tty, fd, 1); + ASSERT(r == 0); + + uv_read_start((uv_stream_t*) &tty, alloc_cb, read_cb); + + // Emulate user-input + str = "got some input\n" + "with a couple of lines\n" + "feel pretty happy\n"; + for (i = 0, len = strlen(str); i < len; i++) { + r = ioctl(fd, TIOCSTI, str + i); + ASSERT(r == 0); + } + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); + + ASSERT(read_count == 3); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + +#endif /* __APPLE__ */ diff --git a/uv.gyp b/uv.gyp index 7824819a9..9bb5b210c 100644 --- a/uv.gyp +++ b/uv.gyp @@ -309,6 +309,7 @@ 'test/test-loop-stop.c', 'test/test-walk-handles.c', 'test/test-multiple-listen.c', + 'test/test-osx-select.c', 'test/test-pass-always.c', 'test/test-ping-pong.c', 'test/test-pipe-bind-error.c', From 4e5b8dc2efb7a5711b0741ff61b9da914b4920fc Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 29 Apr 2013 13:38:57 +0200 Subject: [PATCH 7/9] build: link with libkvm on openbsd The Makefile already did and now the gyp build does too. Fixes the OpenBSD build of node.js. Fixes joyent/node#5363. --- uv.gyp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/uv.gyp b/uv.gyp index 9bb5b210c..17202d1a9 100644 --- a/uv.gyp +++ b/uv.gyp @@ -235,21 +235,16 @@ }], [ 'OS=="freebsd" or OS=="dragonflybsd"', { 'sources': [ 'src/unix/freebsd.c' ], - 'link_settings': { - 'libraries': [ - '-lkvm', - ], - }, }], [ 'OS=="openbsd"', { 'sources': [ 'src/unix/openbsd.c' ], }], [ 'OS=="netbsd"', { 'sources': [ 'src/unix/netbsd.c' ], + }], + [ 'OS in "freebsd dragonflybsd openbsd netbsd".split()', { 'link_settings': { - 'libraries': [ - '-lkvm', - ], + 'libraries': [ '-lkvm' ], }, }], [ 'OS in "mac freebsd dragonflybsd openbsd netbsd".split()', { From 67f9b91a8b76536c47045a9edabe440fa78a42c0 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 30 Apr 2013 12:51:32 +0400 Subject: [PATCH 8/9] stream: use harder sync restrictions for osx-hack Synchronize harder to avoid excessive spins, invokations of async callback and sporadic assertion failures on double-call of async callback. --- src/unix/stream.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/unix/stream.c b/src/unix/stream.c index 485328df2..74bd60a39 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -46,8 +46,8 @@ typedef struct uv__stream_select_s uv__stream_select_t; struct uv__stream_select_s { uv_stream_t* stream; uv_thread_t thread; - uv_sem_t sem; - uv_mutex_t mutex; + uv_sem_t close_sem; + uv_sem_t async_sem; uv_async_t async; int events; int fake_fd; @@ -148,7 +148,7 @@ static void uv__stream_osx_select(void* arg) { while (1) { /* Terminate on semaphore */ - if (uv_sem_trywait(&s->sem) == 0) + if (uv_sem_trywait(&s->close_sem) == 0) break; /* Watch fd using select(2) */ @@ -204,11 +204,13 @@ static void uv__stream_osx_select(void* arg) { assert(events != 0 || FD_ISSET(s->int_fd, &sread)); if (events != 0) { - uv_mutex_lock(&s->mutex); - s->events |= events; + ACCESS_ONCE(int, s->events) = events; uv_async_send(&s->async); - uv_mutex_unlock(&s->mutex); + uv_sem_wait(&s->async_sem); + + /* Should be processed at this stage */ + assert((s->events == 0) || (stream->flags & UV_CLOSING)); } } } @@ -242,10 +244,9 @@ static void uv__stream_osx_select_cb(uv_async_t* handle, int status) { stream = s->stream; /* Get and reset stream's events */ - uv_mutex_lock(&s->mutex); events = s->events; - s->events = 0; - uv_mutex_unlock(&s->mutex); + ACCESS_ONCE(int, s->events) = 0; + uv_sem_post(&s->async_sem); assert(events != 0); assert(events == (events & (UV__POLLIN | UV__POLLOUT))); @@ -318,10 +319,10 @@ int uv__stream_try_select(uv_stream_t* stream, int* fd) { s->async.flags |= UV__HANDLE_INTERNAL; uv__handle_unref(&s->async); - if (uv_sem_init(&s->sem, 0)) + if (uv_sem_init(&s->close_sem, 0)) goto fatal1; - if (uv_mutex_init(&s->mutex)) + if (uv_sem_init(&s->async_sem, 0)) goto fatal2; /* Create fds for io watcher and to interrupt the select() loop. */ @@ -346,9 +347,9 @@ fatal4: s->fake_fd = -1; s->int_fd = -1; fatal3: - uv_mutex_destroy(&s->mutex); + uv_sem_destroy(&s->async_sem); fatal2: - uv_sem_destroy(&s->sem); + uv_sem_destroy(&s->close_sem); fatal1: uv_close((uv_handle_t*) &s->async, uv__stream_osx_cb_close); return uv__set_sys_error(stream->loop, errno); @@ -1360,11 +1361,12 @@ void uv__stream_close(uv_stream_t* handle) { s = handle->select; - uv_sem_post(&s->sem); + uv_sem_post(&s->close_sem); + uv_sem_post(&s->async_sem); uv__stream_osx_interrupt_select(handle); uv_thread_join(&s->thread); - uv_sem_destroy(&s->sem); - uv_mutex_destroy(&s->mutex); + uv_sem_destroy(&s->close_sem); + uv_sem_destroy(&s->async_sem); close(s->fake_fd); close(s->int_fd); uv_close((uv_handle_t*) &s->async, uv__stream_osx_cb_close); From b3ab332b340fb857c4d6bd43208aa708a6eb00bd Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 1 May 2013 19:14:23 +0200 Subject: [PATCH 9/9] unix: fix EMFILE error handling On Linux, the accept() and accept4() system calls checks for EMFILE before checking for EAGAIN. Refine our EMFILE error handling tactic to deal with that. Here is what used to happen: 1. The event loop calls accept() and sees EMFILE. 2. Libuv switches to EMFILE error handling mode. It closes a stashed file descriptor and calls accept() again. 3. The accept() system call fails with EAGAIN. 4. Libuv reopens the stashed file descriptor (reaching RLIMIT_NOFILE again) and returns control to the regular event loop. 5. The regular event loop calls accept(), sees EMFILE and jumps to step 2 again. Effectively an infinite loop. Avoid that by not calling accept() again when we've seen EAGAIN. Fixes joyent/node#5389. --- src/unix/stream.c | 60 ++++++++++++++--------------------------------- 1 file changed, 17 insertions(+), 43 deletions(-) diff --git a/src/unix/stream.c b/src/unix/stream.c index 74bd60a39..fda2f02f5 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -441,7 +441,6 @@ void uv__stream_destroy(uv_stream_t* stream) { */ static int uv__emfile_trick(uv_loop_t* loop, int accept_fd) { int fd; - int r; if (loop->emfile_fd == -1) return -1; @@ -459,14 +458,8 @@ static int uv__emfile_trick(uv_loop_t* loop, int accept_fd) { if (errno == EINTR) continue; - if (errno == EAGAIN || errno == EWOULDBLOCK) - r = 0; - else - r = -1; - - loop->emfile_fd = uv__open_cloexec("/", O_RDONLY); - - return r; + SAVE_ERRNO(loop->emfile_fd = uv__open_cloexec("/", O_RDONLY)); + return errno; } } @@ -479,10 +472,9 @@ static int uv__emfile_trick(uv_loop_t* loop, int accept_fd) { void uv__server_io(uv_loop_t* loop, uv__io_t* w, unsigned int events) { - static int use_emfile_trick = -1; uv_stream_t* stream; + int err; int fd; - int r; stream = container_of(w, uv_stream_t, io_watcher); assert(events == UV__POLLIN); @@ -497,50 +489,32 @@ void uv__server_io(uv_loop_t* loop, uv__io_t* w, unsigned int events) { */ while (uv__stream_fd(stream) != -1) { assert(stream->accepted_fd == -1); + #if defined(UV_HAVE_KQUEUE) if (w->rcount <= 0) return; #endif /* defined(UV_HAVE_KQUEUE) */ + fd = uv__accept(uv__stream_fd(stream)); - if (fd == -1) { - switch (errno) { -#if EWOULDBLOCK != EAGAIN - case EWOULDBLOCK: -#endif - case EAGAIN: - return; /* Not an error. */ + if (errno == EAGAIN || errno == EWOULDBLOCK) + return; /* Not an error. */ - case ECONNABORTED: - UV_DEC_BACKLOG(w) - continue; /* Ignore. */ + if (errno == ECONNABORTED) + continue; /* Ignore. Nothing we can do about that. */ - case EMFILE: - case ENFILE: - if (use_emfile_trick == -1) { - const char* val = getenv("UV_ACCEPT_EMFILE_TRICK"); - use_emfile_trick = (val == NULL || atoi(val) != 0); - } - - if (use_emfile_trick) { - SAVE_ERRNO(r = uv__emfile_trick(loop, uv__stream_fd(stream))); - if (r == 0) { - UV_DEC_BACKLOG(w) - continue; - } - } - - /* Fall through. */ - - default: - uv__set_sys_error(loop, errno); - stream->connection_cb(stream, -1); - continue; + if (errno == EMFILE || errno == ENFILE) { + SAVE_ERRNO(err = uv__emfile_trick(loop, uv__stream_fd(stream))); + if (err == EAGAIN || err == EWOULDBLOCK) + break; } + + uv__set_sys_error(loop, errno); + stream->connection_cb(stream, -1); + continue; } UV_DEC_BACKLOG(w) - stream->accepted_fd = fd; stream->connection_cb(stream, 0);