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. Conflicts: test/test-spawn.c
This commit is contained in:
parent
4f72f2145b
commit
84f305915f
@ -99,6 +99,7 @@ void uv__platform_loop_delete(uv_loop_t* loop) {
|
|||||||
|
|
||||||
void uv__platform_invalidate_fd(uv_loop_t* loop, int fd) {
|
void uv__platform_invalidate_fd(uv_loop_t* loop, int fd) {
|
||||||
struct uv__epoll_event* events;
|
struct uv__epoll_event* events;
|
||||||
|
struct uv__epoll_event dummy;
|
||||||
uintptr_t i;
|
uintptr_t i;
|
||||||
uintptr_t nfds;
|
uintptr_t nfds;
|
||||||
|
|
||||||
@ -106,13 +107,20 @@ void uv__platform_invalidate_fd(uv_loop_t* loop, int fd) {
|
|||||||
|
|
||||||
events = (struct uv__epoll_event*) loop->watchers[loop->nwatchers];
|
events = (struct uv__epoll_event*) loop->watchers[loop->nwatchers];
|
||||||
nfds = (uintptr_t) loop->watchers[loop->nwatchers + 1];
|
nfds = (uintptr_t) loop->watchers[loop->nwatchers + 1];
|
||||||
if (events == NULL)
|
if (events != NULL)
|
||||||
return;
|
|
||||||
|
|
||||||
/* Invalidate events with same file descriptor */
|
/* Invalidate events with same file descriptor */
|
||||||
for (i = 0; i < nfds; i++)
|
for (i = 0; i < nfds; i++)
|
||||||
if ((int) events[i].data == fd)
|
if ((int) events[i].data == fd)
|
||||||
events[i].data = -1;
|
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);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@ -226,6 +226,7 @@ TEST_DECLARE (spawn_setuid_setgid)
|
|||||||
TEST_DECLARE (we_get_signal)
|
TEST_DECLARE (we_get_signal)
|
||||||
TEST_DECLARE (we_get_signals)
|
TEST_DECLARE (we_get_signals)
|
||||||
TEST_DECLARE (signal_multiple_loops)
|
TEST_DECLARE (signal_multiple_loops)
|
||||||
|
TEST_DECLARE (closed_fd_events)
|
||||||
#endif
|
#endif
|
||||||
#ifdef __APPLE__
|
#ifdef __APPLE__
|
||||||
TEST_DECLARE (osx_select)
|
TEST_DECLARE (osx_select)
|
||||||
@ -458,6 +459,7 @@ TASK_LIST_START
|
|||||||
TEST_ENTRY (we_get_signal)
|
TEST_ENTRY (we_get_signal)
|
||||||
TEST_ENTRY (we_get_signals)
|
TEST_ENTRY (we_get_signals)
|
||||||
TEST_ENTRY (signal_multiple_loops)
|
TEST_ENTRY (signal_multiple_loops)
|
||||||
|
TEST_ENTRY (closed_fd_events)
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
#ifdef __APPLE__
|
#ifdef __APPLE__
|
||||||
|
|||||||
@ -40,6 +40,7 @@ static char exepath[1024];
|
|||||||
static size_t exepath_size = 1024;
|
static size_t exepath_size = 1024;
|
||||||
static char* args[3];
|
static char* args[3];
|
||||||
static int no_term_signal;
|
static int no_term_signal;
|
||||||
|
static int timer_counter;
|
||||||
|
|
||||||
#define OUTPUT_SIZE 1024
|
#define OUTPUT_SIZE 1024
|
||||||
static char output[OUTPUT_SIZE];
|
static char output[OUTPUT_SIZE];
|
||||||
@ -118,6 +119,12 @@ static void on_read(uv_stream_t* tcp, ssize_t nread, 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) {
|
static void write_cb(uv_write_t* req, int status) {
|
||||||
ASSERT(status == 0);
|
ASSERT(status == 0);
|
||||||
uv_close((uv_handle_t*)req->handle, close_cb);
|
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) {
|
TEST_IMPL(spawn_fails) {
|
||||||
init_process_options("", exit_cb_failure_expected);
|
init_process_options("", exit_cb_failure_expected);
|
||||||
options.file = options.args[0] = "program-that-had-better-not-exist";
|
options.file = options.args[0] = "program-that-had-better-not-exist";
|
||||||
@ -1038,3 +1050,65 @@ TEST_IMPL(spawn_auto_unref) {
|
|||||||
MAKE_VALGRIND_HAPPY();
|
MAKE_VALGRIND_HAPPY();
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
#ifndef _WIN32
|
||||||
|
TEST_IMPL(closed_fd_events) {
|
||||||
|
uv_stdio_container_t stdio[3];
|
||||||
|
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;
|
||||||
|
options.stdio[0].data.fd = fd[0];
|
||||||
|
options.stdio[0].flags = UV_INHERIT_FD;
|
||||||
|
options.stdio[1].flags = UV_IGNORE;
|
||||||
|
options.stdio[2].flags = UV_IGNORE;
|
||||||
|
|
||||||
|
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)) {
|
||||||
|
if (1 == uv_run(uv_default_loop(), UV_RUN_ONCE))
|
||||||
|
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 */
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user