From 780d8ad8e58c675dc60f5b73ea22a203760da2f2 Mon Sep 17 00:00:00 2001
From: Geoffry Song <goffrie@gmail.com>
Date: Wed, 5 Feb 2014 18:36:24 -0500
Subject: [PATCH] linux: always deregister closing fds from epoll

If the same file description is open in two different processes, then
closing the file descriptor is not sufficient to deregister it from the
epoll instance (as described in epoll(7)), resulting in spurious events
that cause the event loop to spin repeatedly. So always explicitly
deregister it.

Fixes #1099.
---
 src/unix/linux-core.c | 20 ++++++++----
 test/test-list.h      |  2 ++
 test/test-spawn.c     | 74 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 6 deletions(-)

diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c
index f71dd2d6b..97a5126f6 100644
--- a/src/unix/linux-core.c
+++ b/src/unix/linux-core.c
@@ -107,6 +107,7 @@ void uv__platform_loop_delete(uv_loop_t* loop) {
 
 void uv__platform_invalidate_fd(uv_loop_t* loop, int fd) {
   struct uv__epoll_event* events;
+  struct uv__epoll_event dummy;
   uintptr_t i;
   uintptr_t nfds;
 
@@ -114,13 +115,20 @@ void uv__platform_invalidate_fd(uv_loop_t* loop, int fd) {
 
   events = (struct uv__epoll_event*) loop->watchers[loop->nwatchers];
   nfds = (uintptr_t) loop->watchers[loop->nwatchers + 1];
-  if (events == NULL)
-    return;
+  if (events != NULL)
+    /* Invalidate events with same file descriptor */
+    for (i = 0; i < nfds; i++)
+      if ((int) events[i].data == fd)
+        events[i].data = -1;
 
-  /* Invalidate events with same file descriptor */
-  for (i = 0; i < nfds; i++)
-    if ((int) events[i].data == fd)
-      events[i].data = -1;
+  /* Remove the file descriptor from the epoll.
+   * This avoids a problem where the same file description remains open
+   * in another process, causing repeated junk epoll events.
+   *
+   * We pass in a dummy epoll_event, to work around a bug in old kernels.
+   */
+  if (loop->backend_fd >= 0)
+    uv__epoll_ctl(loop->backend_fd, UV__EPOLL_CTL_DEL, fd, &dummy);
 }
 
 
diff --git a/test/test-list.h b/test/test-list.h
index a6e692f63..57511397b 100644
--- a/test/test-list.h
+++ b/test/test-list.h
@@ -240,6 +240,7 @@ TEST_DECLARE   (spawn_setuid_setgid)
 TEST_DECLARE   (we_get_signal)
 TEST_DECLARE   (we_get_signals)
 TEST_DECLARE   (signal_multiple_loops)
+TEST_DECLARE   (closed_fd_events)
 #endif
 #ifdef __APPLE__
 TEST_DECLARE   (osx_select)
@@ -484,6 +485,7 @@ TASK_LIST_START
   TEST_ENTRY  (we_get_signal)
   TEST_ENTRY  (we_get_signals)
   TEST_ENTRY  (signal_multiple_loops)
+  TEST_ENTRY  (closed_fd_events)
 #endif
 
 #ifdef __APPLE__
diff --git a/test/test-spawn.c b/test/test-spawn.c
index e73e8ad86..fc54b4f93 100644
--- a/test/test-spawn.c
+++ b/test/test-spawn.c
@@ -40,6 +40,7 @@ static char exepath[1024];
 static size_t exepath_size = 1024;
 static char* args[3];
 static int no_term_signal;
+static int timer_counter;
 
 #define OUTPUT_SIZE 1024
 static char output[OUTPUT_SIZE];
@@ -118,6 +119,12 @@ static void on_read(uv_stream_t* tcp, ssize_t nread, const uv_buf_t* buf) {
 }
 
 
+static void on_read_once(uv_stream_t* tcp, ssize_t nread, const uv_buf_t* buf) {
+  uv_read_stop(tcp);
+  on_read(tcp, nread, buf);
+}
+
+
 static void write_cb(uv_write_t* req, int status) {
   ASSERT(status == 0);
   uv_close((uv_handle_t*)req->handle, close_cb);
@@ -145,6 +152,11 @@ static void timer_cb(uv_timer_t* handle, int status) {
 }
 
 
+static void timer_counter_cb(uv_timer_t* handle, int status) {
+  ++timer_counter;
+}
+
+
 TEST_IMPL(spawn_fails) {
   int r;
 
@@ -1087,3 +1099,65 @@ TEST_IMPL(spawn_fs_open) {
   return 0;
 }
 #endif  /* !_WIN32 */
+
+
+#ifndef _WIN32
+TEST_IMPL(closed_fd_events) {
+  uv_stdio_container_t stdio[3] = {
+    { UV_INHERIT_FD },
+    { UV_IGNORE },
+    { UV_IGNORE }
+  };
+  uv_pipe_t pipe_handle;
+  int fd[2];
+
+  /* create a pipe and share it with a child process */
+  ASSERT(0 == pipe(fd));
+  ASSERT(0 == fcntl(fd[0], F_SETFL, O_NONBLOCK));
+
+  /* spawn_helper4 blocks indefinitely. */
+  init_process_options("spawn_helper4", exit_cb);
+  options.stdio_count = 3;
+  options.stdio = stdio;
+  stdio[0].data.fd = fd[0];
+
+  ASSERT(0 == uv_spawn(uv_default_loop(), &process, &options));
+  uv_unref((uv_handle_t*) &process);
+
+  /* read from the pipe with uv */
+  ASSERT(0 == uv_pipe_init(uv_default_loop(), &pipe_handle, 0));
+  ASSERT(0 == uv_pipe_open(&pipe_handle, fd[0]));
+  fd[0] = -1;
+
+  ASSERT(0 == uv_read_start((uv_stream_t*) &pipe_handle, on_alloc, on_read_once));
+
+  ASSERT(1 == write(fd[1], "", 1));
+
+  ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_ONCE));
+
+  /* should have received just one byte */
+  ASSERT(output_used == 1);
+
+  /* close the pipe and see if we still get events */
+  uv_close((uv_handle_t*) &pipe_handle, close_cb);
+
+  ASSERT(1 == write(fd[1], "", 1));
+
+  ASSERT(0 == uv_timer_init(uv_default_loop(), &timer));
+  ASSERT(0 == uv_timer_start(&timer, timer_counter_cb, 10, 0));
+
+  /* see if any spurious events interrupt the timer */
+  if (1 == uv_run(uv_default_loop(), UV_RUN_ONCE))
+    /* have to run again to really trigger the timer */
+    ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_ONCE));
+
+  ASSERT(timer_counter == 1);
+
+  /* cleanup */
+  ASSERT(0 == uv_process_kill(&process, /* SIGTERM */ 15));
+  ASSERT(0 == close(fd[1]));
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
+#endif  /* !_WIN32 */