unix: fix accept() EMFILE error handling

Fix a bug that was introduced in commit 3ee4d3f ("unix, windows:
return error codes directly") and add a regression test for good
measure.

Hat tip to Fedor for pointing out the issue.

Fixes #1007.

Makefile.am

@@ -126,6 +126,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \
                          test/test-delayed-accept.c \
                          test/test-dlerror.c \
                          test/test-embed.c \
+                         test/test-emfile.c \
                          test/test-error.c \
                          test/test-fail-always.c \
                          test/test-fs-event.c \

src/unix/stream.c

@@ -462,27 +462,22 @@ void uv__stream_destroy(uv_stream_t* stream) {
  * calling close() and accept().
  */
 static int uv__emfile_trick(uv_loop_t* loop, int accept_fd) {
-  int fd;
+  int err;
 
   if (loop->emfile_fd == -1)
     return -EMFILE;
 
   uv__close(loop->emfile_fd);
+  loop->emfile_fd = -1;
 
-  for (;;) {
+  do {
-    fd = uv__accept(accept_fd);
+    err = uv__accept(accept_fd);
+    if (err >= 0)
+      uv__close(err);
+  } while (err >= 0 || err == -EINTR);
 
-    if (fd != -1) {
+  SAVE_ERRNO(loop->emfile_fd = uv__open_cloexec("/", O_RDONLY));
-      uv__close(fd);
+  return err;
-      continue;
-    }
-
-    if (errno == EINTR)
-      continue;
-
-    SAVE_ERRNO(loop->emfile_fd = uv__open_cloexec("/", O_RDONLY));
-    return -errno;
-  }
 }
 
 

test/test-emfile.c

@@ -0,0 +1,111 @@
+/* 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.
+ */
+
+#if !defined(_WIN32)
+
+#include "uv.h"
+#include "task.h"
+
+#include <errno.h>
+#include <stdio.h>
+#include <sys/resource.h>
+#include <unistd.h>
+
+static void connection_cb(uv_stream_t* server_handle, int status);
+static void connect_cb(uv_connect_t* req, int status);
+
+static const int maxfd = 31;
+static unsigned connect_cb_called;
+static uv_tcp_t server_handle;
+static uv_tcp_t client_handle;
+
+
+TEST_IMPL(emfile) {
+  struct sockaddr_in addr;
+  struct rlimit limits;
+  uv_connect_t connect_req;
+  uv_loop_t* loop;
+  int first_fd;
+
+  loop = uv_default_loop();
+  ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr));
+  ASSERT(0 == uv_tcp_init(loop, &server_handle));
+  ASSERT(0 == uv_tcp_init(loop, &client_handle));
+  ASSERT(0 == uv_tcp_bind(&server_handle, (const struct sockaddr*) &addr));
+  ASSERT(0 == uv_listen((uv_stream_t*) &server_handle, 8, connection_cb));
+
+  /* Lower the file descriptor limit and use up all fds save one. */
+  limits.rlim_cur = limits.rlim_max = maxfd + 1;
+  if (setrlimit(RLIMIT_NOFILE, &limits)) {
+    perror("setrlimit(RLIMIT_NOFILE)");
+    ASSERT(0);
+  }
+
+  /* Remember the first one so we can clean up afterwards. */
+  do
+    first_fd = dup(0);
+  while (first_fd == -1 && errno == EINTR);
+  ASSERT(first_fd > 0);
+
+  while (dup(0) != -1 || errno == EINTR);
+  ASSERT(errno == EMFILE);
+  close(maxfd);
+
+  /* Now connect and use up the last available file descriptor.  The EMFILE
+   * handling logic in src/unix/stream.c should ensure that connect_cb() runs
+   * whereas connection_cb() should *not* run.
+   */
+  ASSERT(0 == uv_tcp_connect(&connect_req,
+                             &client_handle,
+                             (const struct sockaddr*) &addr,
+                             connect_cb));
+  ASSERT(0 == uv_run(loop, UV_RUN_DEFAULT));
+  ASSERT(1 == connect_cb_called);
+
+  /* Close the dups again. Ignore errors in the unlikely event that the
+   * file descriptors were not contiguous.
+   */
+  while (first_fd < maxfd) {
+    close(first_fd);
+    first_fd += 1;
+  }
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
+
+
+static void connection_cb(uv_stream_t* server_handle, int status) {
+  ASSERT(0 && "connection_cb should not be called.");
+}
+
+
+static void connect_cb(uv_connect_t* req, int status) {
+  /* |status| should equal 0 because the connection should have been accepted,
+   * it's just that the server immediately closes it again.
+   */
+  ASSERT(0 == status);
+  connect_cb_called += 1;
+  uv_close((uv_handle_t*) &server_handle, NULL);
+  uv_close((uv_handle_t*) &client_handle, NULL);
+}
+
+#endif  /* !defined(_WIN32) */

test/test-list.h

@@ -225,6 +225,7 @@ TEST_DECLARE   (listen_with_simultaneous_accepts)
 TEST_DECLARE   (listen_no_simultaneous_accepts)
 TEST_DECLARE   (fs_stat_root)
 #else
+TEST_DECLARE   (emfile)
 TEST_DECLARE   (close_fd)
 TEST_DECLARE   (spawn_setuid_setgid)
 TEST_DECLARE   (we_get_signal)
@@ -457,6 +458,7 @@ TASK_LIST_START
   TEST_ENTRY  (listen_no_simultaneous_accepts)
   TEST_ENTRY  (fs_stat_root)
 #else
+  TEST_ENTRY  (emfile)
   TEST_ENTRY  (close_fd)
   TEST_ENTRY  (spawn_setuid_setgid)
   TEST_ENTRY  (we_get_signal)

uv.gyp

@@ -309,6 +309,7 @@
         'test/test-delayed-accept.c',
         'test/test-error.c',
         'test/test-embed.c',
+        'test/test-emfile.c',
         'test/test-fail-always.c',
         'test/test-fs.c',
         'test/test-fs-event.c',