From 29fdb3471ba993a4d129278129ec04e720890063 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sat, 26 Oct 2013 01:06:12 +0400 Subject: [PATCH 1/5] unix: update events from pevents between polls Watchers could be stopped between two `kevent()`/`epoll_wait()` calls (which may happen in the same loop in `uv__io_poll()`), in such cases `watcher->events` could be stale and won't be updated to `watcher->pevents`. Try to use and rely on `watcher->pevents` instead of blindly expecting `watcher->events` to be always correct. --- src/unix/kqueue.c | 4 ++-- src/unix/linux-core.c | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/unix/kqueue.c b/src/unix/kqueue.c index 378903a6..d99d6e1b 100644 --- a/src/unix/kqueue.c +++ b/src/unix/kqueue.c @@ -191,7 +191,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { revents = 0; if (ev->filter == EVFILT_READ) { - if (w->events & UV__POLLIN) { + if (w->pevents & UV__POLLIN) { revents |= UV__POLLIN; w->rcount = ev->data; } else { @@ -205,7 +205,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { } if (ev->filter == EVFILT_WRITE) { - if (w->events & UV__POLLOUT) { + if (w->pevents & UV__POLLOUT) { revents |= UV__POLLOUT; w->wcount = ev->data; } else { diff --git a/src/unix/linux-core.c b/src/unix/linux-core.c index e4c34a18..cb6df789 100644 --- a/src/unix/linux-core.c +++ b/src/unix/linux-core.c @@ -105,6 +105,7 @@ 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; @@ -208,7 +209,15 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { continue; } - w->cb(loop, w, pe->events); + /* + * 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++; } From f9960184fd86d43470dc70dcedac32899e532757 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 30 Oct 2013 21:43:23 +0400 Subject: [PATCH 2/5] test: add regression test for 29fdb3471 --- build.mk | 1 + checksparse.sh | 1 + test/test-list.h | 3 + test/test-watcher-cross-stop.c | 108 +++++++++++++++++++++++++++++++++ uv.gyp | 1 + 5 files changed, 114 insertions(+) create mode 100644 test/test-watcher-cross-stop.c diff --git a/build.mk b/build.mk index 6cba169f..2b86ba0e 100644 --- a/build.mk +++ b/build.mk @@ -142,6 +142,7 @@ TESTS= \ test/test-udp-send-and-recv.o \ test/test-util.o \ test/test-walk-handles.o \ + test/test-watcher-cross-stop.o \ .PHONY: all bench clean clean-platform distclean test diff --git a/checksparse.sh b/checksparse.sh index 7412c21c..4a508e26 100755 --- a/checksparse.sh +++ b/checksparse.sh @@ -161,6 +161,7 @@ test/test-udp-options.c test/test-udp-send-and-recv.c test/test-util.c test/test-walk-handles.c +test/test-watcher-cross-stop.c " case `uname -s` in diff --git a/test/test-list.h b/test/test-list.h index 7a9b1a29..d883496f 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -106,6 +106,7 @@ TEST_DECLARE (idle_starvation) TEST_DECLARE (loop_handles) TEST_DECLARE (get_loadavg) TEST_DECLARE (walk_handles) +TEST_DECLARE (watcher_cross_stop) TEST_DECLARE (ref) TEST_DECLARE (idle_ref) TEST_DECLARE (async_ref) @@ -385,6 +386,8 @@ TASK_LIST_START TEST_ENTRY (loop_handles) TEST_ENTRY (walk_handles) + TEST_ENTRY (watcher_cross_stop) + TEST_ENTRY (active) TEST_ENTRY (embed) diff --git a/test/test-watcher-cross-stop.c b/test/test-watcher-cross-stop.c new file mode 100644 index 00000000..5b53320b --- /dev/null +++ b/test/test-watcher-cross-stop.c @@ -0,0 +1,108 @@ +/* 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. + */ + +#include "uv.h" +#include "task.h" + +#include +#include + +#if !defined(_WIN32) +#include +#include /* setrlimit() */ +#endif + +/* NOTE: Number should be big enough to trigger this problem */ +static uv_udp_t sockets[2500]; +static uv_udp_send_t reqs[ARRAY_SIZE(sockets)]; +static char buf[1]; +static unsigned int recv_cb_called; +static unsigned int send_cb_called; +static unsigned int close_cb_called; + +static uv_buf_t alloc_cb(uv_handle_t* handle, size_t suggested_size) { + return uv_buf_init(buf, sizeof(buf)); +} + + +static void recv_cb(uv_udp_t* handle, + ssize_t nread, + uv_buf_t buf, + struct sockaddr* addr, + unsigned flags) { + recv_cb_called++; +} + + +static void send_cb(uv_udp_send_t* req, int status) { + send_cb_called++; +} + + +static void close_cb(uv_handle_t* handle) { + close_cb_called++; +} + + +TEST_IMPL(watcher_cross_stop) { + uv_loop_t* loop = uv_default_loop(); + unsigned int i; + struct sockaddr_in addr; + uv_buf_t buf; + char big_string[1024]; + +#if !defined(_WIN32) + { + struct rlimit lim; + lim.rlim_cur = ARRAY_SIZE(sockets) + 32; + lim.rlim_max = ARRAY_SIZE(sockets) + 32; + if (setrlimit(RLIMIT_NOFILE, &lim)) + RETURN_SKIP("File descriptor limit too low."); + } +#endif + + addr = uv_ip4_addr("127.0.0.1", TEST_PORT); + memset(big_string, 'A', sizeof(big_string)); + buf = uv_buf_init(big_string, sizeof(big_string)); + + for (i = 0; i < ARRAY_SIZE(sockets); i++) { + ASSERT(0 == uv_udp_init(loop, &sockets[i])); + ASSERT(0 == uv_udp_bind(&sockets[i], addr, 0)); + ASSERT(0 == uv_udp_recv_start(&sockets[i], alloc_cb, recv_cb)); + ASSERT(0 == uv_udp_send(&reqs[i], &sockets[i], &buf, 1, addr, send_cb)); + } + + while (recv_cb_called == 0) + uv_run(loop, UV_RUN_ONCE); + + for (i = 0; i < ARRAY_SIZE(sockets); i++) + uv_close((uv_handle_t*) &sockets[i], close_cb); + + ASSERT(0 < recv_cb_called && recv_cb_called <= ARRAY_SIZE(sockets)); + ASSERT(ARRAY_SIZE(sockets) == send_cb_called); + + uv_run(loop, UV_RUN_DEFAULT); + + ASSERT(ARRAY_SIZE(sockets) == close_cb_called); + + MAKE_VALGRIND_HAPPY(); + return 0; +} diff --git a/uv.gyp b/uv.gyp index f61ebb53..7d43d87e 100644 --- a/uv.gyp +++ b/uv.gyp @@ -310,6 +310,7 @@ 'test/test-loop-handles.c', 'test/test-loop-stop.c', 'test/test-walk-handles.c', + 'test/test-watcher-cross-stop.c', 'test/test-multiple-listen.c', 'test/test-osx-select.c', 'test/test-pass-always.c', From 3780e128230618bc3bddc9b3942ee149fadf4ca2 Mon Sep 17 00:00:00 2001 From: Chris Bank Date: Sat, 2 Nov 2013 05:00:50 +0400 Subject: [PATCH 3/5] fsevents: support japaneese characters in path --- src/unix/fsevents.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/unix/fsevents.c b/src/unix/fsevents.c index b6d27467..d8603242 100644 --- a/src/unix/fsevents.c +++ b/src/unix/fsevents.c @@ -223,9 +223,7 @@ int uv__fsevents_init(uv_fs_event_t* handle) { handle->realpath_len = strlen(handle->realpath); /* Initialize paths array */ - path = CFStringCreateWithCString(NULL, - handle->filename, - CFStringGetSystemEncoding()); + path = CFStringCreateWithFileSystemRepresentation(NULL, handle->filename); paths = CFArrayCreate(NULL, (const void**)&path, 1, NULL); latency = 0.15; From 3d2c820a4efe3954a77b539bb56e7398263069d3 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 30 Oct 2013 09:36:47 +0100 Subject: [PATCH 4/5] linux: don't turn on SO_REUSEPORT socket option On the BSDs, SO_REUSEPORT is pretty much SO_REUSEADDR with some special casing for IP multicast. When two processes (that don't do multicast) bind to the same address, only the last one receives traffic. It allows one to "steal" the bound address from another process. (Both processes have to enable SO_REUSEPORT though, so it only works in a cooperative setting.) On Linux however, it enables port sharing, not stealing - both processes receive a share of the traffic. This is a desirable trait but pre-3.9 kernels don't support the socket option and a libuv program therefore behaves differently with older kernels or on another platform. This is a back-port of commit 9d60f1e from the master branch. Fixes joyent/node#6454. --- src/unix/udp.c | 48 +++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/src/unix/udp.c b/src/unix/udp.c index 3fb8af93..ea437a3e 100644 --- a/src/unix/udp.c +++ b/src/unix/udp.c @@ -286,6 +286,31 @@ static void uv__udp_sendmsg(uv_loop_t* loop, } +/* On the BSDs, SO_REUSEPORT implies SO_REUSEADDR but with some additional + * refinements for programs that use multicast. + * + * Linux as of 3.9 has a SO_REUSEPORT socket option but with semantics that + * are different from the BSDs: it _shares_ the port rather than steal it + * from the current listener. While useful, it's not something we can emulate + * on other platforms so we don't enable it. + */ +static int uv__set_reuse(int fd) { + int yes; + +#if defined(SO_REUSEPORT) && !defined(__linux__) + yes = 1; + if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &yes, sizeof(yes))) + return -errno; +#else + yes = 1; + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes))) + return -errno; +#endif + + return 0; +} + + static int uv__bind(uv_udp_t* handle, int domain, struct sockaddr* addr, @@ -293,6 +318,7 @@ static int uv__bind(uv_udp_t* handle, unsigned flags) { int saved_errno; int status; + int err; int yes; int fd; @@ -321,28 +347,12 @@ static int uv__bind(uv_udp_t* handle, } fd = handle->io_watcher.fd; - yes = 1; - if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof yes) == -1) { - uv__set_sys_error(handle->loop, errno); + err = uv__set_reuse(fd); + if (err) { + uv__set_sys_error(handle->loop, -err); goto out; } - /* On the BSDs, SO_REUSEADDR lets you reuse an address that's in the TIME_WAIT - * state (i.e. was until recently tied to a socket) while SO_REUSEPORT lets - * multiple processes bind to the same address. Yes, it's something of a - * misnomer but then again, SO_REUSEADDR was already taken. - * - * None of the above applies to Linux: SO_REUSEADDR implies SO_REUSEPORT on - * Linux and hence it does not have SO_REUSEPORT at all. - */ -#ifdef SO_REUSEPORT - yes = 1; - if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &yes, sizeof yes) == -1) { - uv__set_sys_error(handle->loop, errno); - goto out; - } -#endif - if (flags & UV_UDP_IPV6ONLY) { #ifdef IPV6_V6ONLY yes = 1; From 991409e461e3973f006bf951b3e7513578a41be2 Mon Sep 17 00:00:00 2001 From: Geert Jansen Date: Tue, 5 Nov 2013 08:43:40 +0100 Subject: [PATCH 5/5] build: fix windows smp build with gyp Gyp will try a parallel build if it detect the system has >1 processor. This functionality depends on the Python "multiprocessing" package. The multiprocessing package on Windows requires that the top-level module is importable as a module, see: http://docs.python.org/2/library/multiprocessing.html#windows This fixes issue #984. This is a back-port of commit 2445467 from the master branch. --- README.md | 4 ++-- gyp_uv => gyp_uv.py | 0 vcbuild.bat | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename gyp_uv => gyp_uv.py (100%) diff --git a/README.md b/README.md index 54740ca9..1e1aa25c 100644 --- a/README.md +++ b/README.md @@ -91,12 +91,12 @@ Or: Unix users run - ./gyp_uv -f make + ./gyp_uv.py -f make make -C out Macintosh users run - ./gyp_uv -f xcode + ./gyp_uv.py -f xcode xcodebuild -project uv.xcodeproj -configuration Release -target All Note for UNIX users: compile your project with `-D_LARGEFILE_SOURCE` and diff --git a/gyp_uv b/gyp_uv.py similarity index 100% rename from gyp_uv rename to gyp_uv.py diff --git a/vcbuild.bat b/vcbuild.bat index 1b2f865a..0b7ea481 100644 --- a/vcbuild.bat +++ b/vcbuild.bat @@ -91,7 +91,7 @@ exit /b 1 :have_gyp if not defined PYTHON set PYTHON="python" -%PYTHON% gyp_uv -Dtarget_arch=%target_arch% -Dlibrary=%library% +%PYTHON% gyp_uv.py -Dtarget_arch=%target_arch% -Dlibrary=%library% if errorlevel 1 goto create-msvs-files-failed if not exist uv.sln goto create-msvs-files-failed echo Project files generated.