Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Commit

Permalink
linux: always deregister closing fds from epoll
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
goffrie authored and indutny committed Feb 19, 2014
1 parent f17c535 commit 780d8ad
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 7 deletions.
22 changes: 15 additions & 7 deletions src/unix/linux-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,28 @@ 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;

assert(loop->watchers != NULL);

events = (struct uv__epoll_event*) loop->watchers[loop->nwatchers];
nfds = (uintptr_t) loop->watchers[loop->nwatchers + 1];
if (events == NULL)
return;

/* Invalidate events with same file descriptor */
for (i = 0; i < nfds; i++)
if ((int) events[i].data == fd)
events[i].data = -1;
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;

/* 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);
}


Expand Down
2 changes: 2 additions & 0 deletions test/test-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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__
Expand Down
74 changes: 74 additions & 0 deletions test/test-spawn.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 */

0 comments on commit 780d8ad

Please sign in to comment.