From 2f55353490506ed08709c65205bedeb67342b194 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sun, 6 Jan 2013 17:55:51 +0100 Subject: [PATCH] unix: update loop->time after poll Fixes a bug where timers expire prematurely when the following conditions hold: a) libuv first spends some time blocked in the platform poll function b) a callback then calls uv_timer_start() Cause: uv_timer_start() uses an out-of-date loop->time in its 'when should the timer callback run?' calculations. Solution: Update loop->time before invoking any callbacks. Fixes #678. --- src/unix/core.c | 4 ++-- src/unix/internal.h | 21 +++++++++++++-------- src/unix/kqueue.c | 11 +++++++---- src/unix/linux/linux-core.c | 11 +++++++---- src/unix/sunos.c | 11 +++++++---- 5 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/unix/core.c b/src/unix/core.c index def4a892..bfc98046 100644 --- a/src/unix/core.c +++ b/src/unix/core.c @@ -286,7 +286,7 @@ int uv_run2(uv_loop_t* loop, uv_run_mode mode) { return 0; do { - uv_update_time(loop); + uv__update_time(loop); uv__run_timers(loop); uv__run_idle(loop); uv__run_prepare(loop); @@ -307,7 +307,7 @@ int uv_run(uv_loop_t* loop) { void uv_update_time(uv_loop_t* loop) { - loop->time = uv__hrtime() / 1000000; + uv__update_time(loop); } diff --git a/src/unix/internal.h b/src/unix/internal.h index 5f04dc29..b3c05771 100644 --- a/src/unix/internal.h +++ b/src/unix/internal.h @@ -105,14 +105,6 @@ enum { UV_TCP_SINGLE_ACCEPT = 0x400 /* Only accept() when idle. */ }; -__attribute__((unused)) -static void uv__req_init(uv_loop_t* loop, uv_req_t* req, uv_req_type type) { - req->type = type; - uv__req_register(loop, req); -} -#define uv__req_init(loop, req, type) \ - uv__req_init((loop), (uv_req_t*)(req), (type)) - /* core */ void uv__handle_init(uv_loop_t* loop, uv_handle_t* handle, uv_handle_type type); int uv__nonblock(int fd, int set); @@ -238,4 +230,17 @@ static const int kFSEventStreamEventFlagItemIsSymlink = 0x00040000; #endif /* defined(__APPLE__) */ +__attribute__((unused)) +static void uv__req_init(uv_loop_t* loop, uv_req_t* req, uv_req_type type) { + req->type = type; + uv__req_register(loop, req); +} +#define uv__req_init(loop, req, type) \ + uv__req_init((loop), (uv_req_t*)(req), (type)) + +__attribute__((unused)) +static void uv__update_time(uv_loop_t* loop) { + loop->time = uv__hrtime() / 1000000; +} + #endif /* UV_UNIX_INTERNAL_H_ */ diff --git a/src/unix/kqueue.c b/src/unix/kqueue.c index 50208901..2ab0854b 100644 --- a/src/unix/kqueue.c +++ b/src/unix/kqueue.c @@ -141,6 +141,12 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { ARRAY_SIZE(events), timeout == -1 ? NULL : &spec); + /* Update loop->time unconditionally. It's tempting to skip the update when + * timeout == 0 (i.e. non-blocking poll) but there is no guarantee that the + * operating system didn't reschedule our process while in the syscall. + */ + SAVE_ERRNO(uv__update_time(loop)); + if (nfds == 0) { assert(timeout != -1); return; @@ -244,10 +250,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { update_timeout: assert(timeout > 0); - diff = uv__hrtime() / 1000000; - assert(diff >= base); - diff -= base; - + diff = loop->time - base; if (diff >= (uint64_t) timeout) return; diff --git a/src/unix/linux/linux-core.c b/src/unix/linux/linux-core.c index 28e1141f..0fec63d1 100644 --- a/src/unix/linux/linux-core.c +++ b/src/unix/linux/linux-core.c @@ -182,6 +182,12 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { ARRAY_SIZE(events), timeout); + /* Update loop->time unconditionally. It's tempting to skip the update when + * timeout == 0 (i.e. non-blocking poll) but there is no guarantee that the + * operating system didn't reschedule our process while in the syscall. + */ + SAVE_ERRNO(uv__update_time(loop)); + if (nfds == 0) { assert(timeout != -1); return; @@ -243,10 +249,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { update_timeout: assert(timeout > 0); - diff = uv__hrtime() / 1000000; - assert(diff >= base); - diff -= base; - + diff = loop->time - base; if (diff >= (uint64_t) timeout) return; diff --git a/src/unix/sunos.c b/src/unix/sunos.c index db5a9c8b..3fbb50c9 100644 --- a/src/unix/sunos.c +++ b/src/unix/sunos.c @@ -150,6 +150,12 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { abort(); } + /* Update loop->time unconditionally. It's tempting to skip the update when + * timeout == 0 (i.e. non-blocking poll) but there is no guarantee that the + * operating system didn't reschedule our process while in the syscall. + */ + SAVE_ERRNO(uv__update_time(loop)); + if (events[0].portev_source == 0) { if (timeout == 0) return; @@ -211,10 +217,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { update_timeout: assert(timeout > 0); - diff = uv__hrtime() / 1000000; - assert(diff >= base); - diff -= base; - + diff = loop->time - base; if (diff >= (uint64_t) timeout) return;