diff --git a/Makefile.am b/Makefile.am index 89a0c252..660c3dfc 100644 --- a/Makefile.am +++ b/Makefile.am @@ -118,6 +118,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \ test/test-barrier.c \ test/test-callback-order.c \ test/test-callback-stack.c \ + test/test-close-fd.c \ test/test-close-order.c \ test/test-condvar.c \ test/test-connection-fail.c \ diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c index 99f914bf..4ad65e90 100644 --- a/src/unix/linux-core.c +++ b/src/unix/linux-core.c @@ -113,7 +113,6 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { uv__io_t* w; uint64_t base; uint64_t diff; - unsigned int masked_events; int nevents; int count; int nfds; @@ -217,16 +216,35 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { continue; } - /* - * Give users only events they're interested in. Prevents spurious + /* Give users only events they're interested in. Prevents spurious * callbacks when previous callback invocation in this loop has stopped * the current watcher. Also, filters out events that users has not * requested us to watch. */ - masked_events = pe->events & w->pevents; - if (masked_events != 0) - w->cb(loop, w, masked_events); - nevents++; + pe->events &= w->pevents | UV__POLLERR | UV__POLLHUP; + + /* Work around an epoll quirk where it sometimes reports just the + * EPOLLERR or EPOLLHUP event. In order to force the event loop to + * move forward, we merge in the read/write events that the watcher + * is interested in; uv__read() and uv__write() will then deal with + * the error or hangup in the usual fashion. + * + * Note to self: happens when epoll reports EPOLLIN|EPOLLHUP, the user + * reads the available data, calls uv_read_stop(), then sometime later + * calls uv_read_start() again. By then, libuv has forgotten about the + * hangup and the kernel won't report EPOLLIN again because there's + * nothing left to read. If anything, libuv is to blame here. The + * current hack is just a quick bandaid; to properly fix it, libuv + * needs to remember the error/hangup event. We should get that for + * free when we switch over to edge-triggered I/O. + */ + if (pe->events == UV__EPOLLERR || pe->events == UV__EPOLLHUP) + pe->events |= w->pevents & (UV__EPOLLIN | UV__EPOLLOUT); + + if (pe->events != 0) { + w->cb(loop, w, pe->events); + nevents++; + } } if (nevents != 0) { diff --git a/test/test-close-fd.c b/test/test-close-fd.c new file mode 100644 index 00000000..0d17f076 --- /dev/null +++ b/test/test-close-fd.c @@ -0,0 +1,77 @@ +/* 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 +#include + +static unsigned int read_cb_called; + +static void alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) { + static char slab[1]; + buf->base = slab; + buf->len = sizeof(slab); +} + +static void read_cb(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf) { + switch (++read_cb_called) { + case 1: + ASSERT(nread == 1); + uv_read_stop(handle); + break; + case 2: + ASSERT(nread == UV_EOF); + uv_close((uv_handle_t *) handle, NULL); + break; + default: + ASSERT(!"read_cb_called > 2"); + } +} + +TEST_IMPL(close_fd) { + uv_pipe_t pipe_handle; + int fd[2]; + + ASSERT(0 == pipe(fd)); + ASSERT(0 == fcntl(fd[0], F_SETFL, O_NONBLOCK)); + ASSERT(0 == uv_pipe_init(uv_default_loop(), &pipe_handle, 0)); + ASSERT(0 == uv_pipe_open(&pipe_handle, fd[0])); + fd[0] = -1; /* uv_pipe_open() takes ownership of the file descriptor. */ + ASSERT(1 == write(fd[1], "", 1)); + ASSERT(0 == close(fd[1])); + fd[1] = -1; + ASSERT(0 == uv_read_start((uv_stream_t *) &pipe_handle, alloc_cb, read_cb)); + ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT)); + ASSERT(1 == read_cb_called); + ASSERT(0 == uv_is_active((const uv_handle_t *) &pipe_handle)); + ASSERT(0 == uv_read_start((uv_stream_t *) &pipe_handle, alloc_cb, read_cb)); + ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT)); + ASSERT(2 == read_cb_called); + ASSERT(0 != uv_is_closing((const uv_handle_t *) &pipe_handle)); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + +#endif /* !defined(_WIN32) */ diff --git a/test/test-list.h b/test/test-list.h index edcbdb96..38fd54db 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -223,6 +223,7 @@ TEST_DECLARE (listen_with_simultaneous_accepts) TEST_DECLARE (listen_no_simultaneous_accepts) TEST_DECLARE (fs_stat_root) #else +TEST_DECLARE (close_fd) TEST_DECLARE (spawn_setuid_setgid) TEST_DECLARE (we_get_signal) TEST_DECLARE (we_get_signals) @@ -453,6 +454,7 @@ TASK_LIST_START TEST_ENTRY (listen_no_simultaneous_accepts) TEST_ENTRY (fs_stat_root) #else + TEST_ENTRY (close_fd) TEST_ENTRY (spawn_setuid_setgid) TEST_ENTRY (we_get_signal) TEST_ENTRY (we_get_signals) diff --git a/uv.gyp b/uv.gyp index 54102af2..0b77203b 100644 --- a/uv.gyp +++ b/uv.gyp @@ -302,6 +302,7 @@ 'test/test-async-null-cb.c', 'test/test-callback-stack.c', 'test/test-callback-order.c', + 'test/test-close-fd.c', 'test/test-close-order.c', 'test/test-connection-fail.c', 'test/test-cwd-and-chdir.c',