diff --git a/docs/src/design.rst b/docs/src/design.rst index 0f5580c7..5a20595c 100644 --- a/docs/src/design.rst +++ b/docs/src/design.rst @@ -60,16 +60,15 @@ stages of a loop iteration: :align: center -#. The loop concept of 'now' is updated. The event loop caches the current time at the start of - the event loop tick in order to reduce the number of time-related system calls. +#. The loop concept of 'now' is initially set. + +#. Due timers are run if the loop was run with ``UV_RUN_DEFAULT``. All active timers scheduled + for a time before the loop's concept of *now* get their callbacks called. #. If the loop is *alive* an iteration is started, otherwise the loop will exit immediately. So, when is a loop considered to be *alive*? If a loop has active and ref'd handles, active requests or closing handles it's considered to be *alive*. -#. Due timers are run. All active timers scheduled for a time before the loop's concept of *now* - get their callbacks called. - #. Pending callbacks are called. All I/O callbacks are called right after polling for I/O, for the most part. There are cases, however, in which calling such a callback is deferred for the next loop iteration. If the previous iteration deferred any I/O callback it will be run at this point. @@ -101,9 +100,11 @@ stages of a loop iteration: #. Close callbacks are called. If a handle was closed by calling :c:func:`uv_close` it will get the close callback called. -#. Special case in case the loop was run with ``UV_RUN_ONCE``, as it implies forward progress. - It's possible that no I/O callbacks were fired after blocking for I/O, but some time has passed - so there might be timers which are due, those timers get their callbacks called. +#. The loop concept of 'now' is updated. + +#. Due timers are run. Note that 'now' is not updated again until the next loop iteration. + So if a timer became due while other timers were being processed, it won't be run until + the following event loop iteration. #. Iteration ends. If the loop was run with ``UV_RUN_NOWAIT`` or ``UV_RUN_ONCE`` modes the iteration ends and :c:func:`uv_run` will return. If the loop was run with ``UV_RUN_DEFAULT`` diff --git a/docs/src/static/loop_iteration.png b/docs/src/static/loop_iteration.png index e769cf33..1545f84a 100644 Binary files a/docs/src/static/loop_iteration.png and b/docs/src/static/loop_iteration.png differ diff --git a/src/unix/core.c b/src/unix/core.c index 8f7ff943..9343ee72 100644 --- a/src/unix/core.c +++ b/src/unix/core.c @@ -390,10 +390,17 @@ int uv_run(uv_loop_t* loop, uv_run_mode mode) { if (!r) uv__update_time(loop); - while (r != 0 && loop->stop_flag == 0) { - uv__update_time(loop); + /* Maintain backwards compatibility by processing timers before entering the + * while loop for UV_RUN_DEFAULT. Otherwise timers only need to be executed + * once, which should be done after polling in order to maintain proper + * execution order of the conceptual event loop. */ + if (mode == UV_RUN_DEFAULT) { + if (r) + uv__update_time(loop); uv__run_timers(loop); + } + while (r != 0 && loop->stop_flag == 0) { can_sleep = QUEUE_EMPTY(&loop->pending_queue) && QUEUE_EMPTY(&loop->idle_handles); @@ -424,18 +431,8 @@ int uv_run(uv_loop_t* loop, uv_run_mode mode) { uv__run_check(loop); uv__run_closing_handles(loop); - if (mode == UV_RUN_ONCE) { - /* UV_RUN_ONCE implies forward progress: at least one callback must have - * been invoked when it returns. uv__io_poll() can return without doing - * I/O (meaning: no callbacks) when its timeout expires - which means we - * have pending timers that satisfy the forward progress constraint. - * - * UV_RUN_NOWAIT makes no guarantees about progress so it's omitted from - * the check. - */ - uv__update_time(loop); - uv__run_timers(loop); - } + uv__update_time(loop); + uv__run_timers(loop); r = uv__loop_alive(loop); if (mode == UV_RUN_ONCE || mode == UV_RUN_NOWAIT) diff --git a/src/win/core.c b/src/win/core.c index 09a3c7b1..a52af5a1 100644 --- a/src/win/core.c +++ b/src/win/core.c @@ -609,10 +609,17 @@ int uv_run(uv_loop_t *loop, uv_run_mode mode) { if (!r) uv_update_time(loop); - while (r != 0 && loop->stop_flag == 0) { - uv_update_time(loop); + /* Maintain backwards compatibility by processing timers before entering the + * while loop for UV_RUN_DEFAULT. Otherwise timers only need to be executed + * once, which should be done after polling in order to maintain proper + * execution order of the conceptual event loop. */ + if (mode == UV_RUN_DEFAULT) { + if (r) + uv_update_time(loop); uv__run_timers(loop); + } + while (r != 0 && loop->stop_flag == 0) { can_sleep = loop->pending_reqs_tail == NULL && loop->idle_handles == NULL; uv__process_reqs(loop); @@ -645,18 +652,8 @@ int uv_run(uv_loop_t *loop, uv_run_mode mode) { uv__check_invoke(loop); uv__process_endgames(loop); - if (mode == UV_RUN_ONCE) { - /* UV_RUN_ONCE implies forward progress: at least one callback must have - * been invoked when it returns. uv__io_poll() can return without doing - * I/O (meaning: no callbacks) when its timeout expires - which means we - * have pending timers that satisfy the forward progress constraint. - * - * UV_RUN_NOWAIT makes no guarantees about progress so it's omitted from - * the check. - */ - uv_update_time(loop); - uv__run_timers(loop); - } + uv_update_time(loop); + uv__run_timers(loop); r = uv__loop_alive(loop); if (mode == UV_RUN_ONCE || mode == UV_RUN_NOWAIT) diff --git a/test/test-list.h b/test/test-list.h index 4cd06356..13c96d1d 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -233,6 +233,8 @@ TEST_DECLARE (timer_from_check) TEST_DECLARE (timer_is_closing) TEST_DECLARE (timer_null_callback) TEST_DECLARE (timer_early_check) +TEST_DECLARE (timer_no_double_call_once) +TEST_DECLARE (timer_no_double_call_nowait) TEST_DECLARE (idle_starvation) TEST_DECLARE (idle_check) TEST_DECLARE (loop_handles) @@ -842,6 +844,8 @@ TASK_LIST_START TEST_ENTRY (timer_is_closing) TEST_ENTRY (timer_null_callback) TEST_ENTRY (timer_early_check) + TEST_ENTRY (timer_no_double_call_once) + TEST_ENTRY (timer_no_double_call_nowait) TEST_ENTRY (idle_starvation) TEST_ENTRY (idle_check) diff --git a/test/test-timer.c b/test/test-timer.c index 1753fbcc..eb54bb25 100644 --- a/test/test-timer.c +++ b/test/test-timer.c @@ -30,6 +30,7 @@ static int twice_close_cb_called = 0; static int repeat_cb_called = 0; static int repeat_close_cb_called = 0; static int order_cb_called = 0; +static int timer_check_double_call_called = 0; static uint64_t start_time; static uv_timer_t tiny_timer; static uv_timer_t huge_timer1; @@ -368,3 +369,41 @@ TEST_IMPL(timer_early_check) { MAKE_VALGRIND_HAPPY(uv_default_loop()); return 0; } + +static void timer_check_double_call(uv_timer_t* handle) { + timer_check_double_call_called++; +} + +TEST_IMPL(timer_no_double_call_once) { + uv_timer_t timer_handle; + const uint64_t timeout_ms = 10; + + ASSERT_EQ(0, uv_timer_init(uv_default_loop(), &timer_handle)); + ASSERT_EQ(0, uv_timer_start(&timer_handle, + timer_check_double_call, + timeout_ms, + timeout_ms)); + uv_sleep(timeout_ms * 2); + ASSERT_EQ(1, uv_run(uv_default_loop(), UV_RUN_ONCE)); + ASSERT_EQ(1, timer_check_double_call_called); + + MAKE_VALGRIND_HAPPY(uv_default_loop()); + return 0; +} + +TEST_IMPL(timer_no_double_call_nowait) { + uv_timer_t timer_handle; + const uint64_t timeout_ms = 10; + + ASSERT_EQ(0, uv_timer_init(uv_default_loop(), &timer_handle)); + ASSERT_EQ(0, uv_timer_start(&timer_handle, + timer_check_double_call, + timeout_ms, + timeout_ms)); + uv_sleep(timeout_ms * 2); + ASSERT_EQ(1, uv_run(uv_default_loop(), UV_RUN_NOWAIT)); + ASSERT_EQ(1, timer_check_double_call_called); + + MAKE_VALGRIND_HAPPY(uv_default_loop()); + return 0; +}