c/libuv
1
0
Fork 0
mirror of https://github.com/libuv/libuv synced 2025-03-28 21:13:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

unix,win: fix busy loop with zero timeout timers (#4250)

Browse Source 
Calling `uv_timer_start(h, cb, 0, 0)` from a timer callback resulted in
the timer running immediately because it was inserted at the front of
the timer heap.

If the callback did that every time, libuv would effectively busy-loop
in `uv__run_timers()` and never make forward progress.

Work around that by collecting all expired timers into a queue and only
running their callback afterwards.

Fixes: https://github.com/libuv/libuv/issues/4245
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
This commit is contained in:
Matheus Izvekov 2023-12-22 08:40:50 -03:00 committed by GitHub
parent 8a499e1331
commit 51a22f60d6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 67 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
include/uv/unix.h
View File

@ -328,7 +328,10 @@ typedef struct {
#define UV_TIMER_PRIVATE_FIELDS \
uv_timer_cb timer_cb; \
void* heap_node[3]; \
union { \
void* heap[3]; \
struct uv__queue queue; \
} node; \
uint64_t timeout; \
uint64_t repeat; \
uint64_t start_id;

5
include/uv/win.h
View File

@ -550,7 +550,10 @@ typedef struct {
unsigned char events;
#define UV_TIMER_PRIVATE_FIELDS \
void* heap_node[3]; \
union { \
void* heap[3]; \
struct uv__queue queue; \
} node; \
int unused; \
uint64_t timeout; \
uint64_t repeat; \

45
src/timer.c
View File

@ -40,8 +40,8 @@ static int timer_less_than(const struct heap_node* ha,
const uv_timer_t* a;
const uv_timer_t* b;
a = container_of(ha, uv_timer_t, heap_node);
b = container_of(hb, uv_timer_t, heap_node);
a = container_of(ha, uv_timer_t, node.heap);
b = container_of(hb, uv_timer_t, node.heap);
if (a->timeout < b->timeout)
return 1;
@ -60,6 +60,7 @@ int uv_timer_init(uv_loop_t* loop, uv_timer_t* handle) {
handle->timer_cb = NULL;
handle->timeout = 0;
handle->repeat = 0;
uv__queue_init(&handle->node.queue);
return 0;
}
@ -73,8 +74,7 @@ int uv_timer_start(uv_timer_t* handle,
if (uv__is_closing(handle) || cb == NULL)
return UV_EINVAL;
if (uv__is_active(handle))
uv_timer_stop(handle);
uv_timer_stop(handle);
clamped_timeout = handle->loop->time + timeout;
if (clamped_timeout < timeout)
@ -87,23 +87,27 @@ int uv_timer_start(uv_timer_t* handle,
handle->start_id = handle->loop->timer_counter++;
heap_insert(timer_heap(handle->loop),
(struct heap_node*) &handle->heap_node,
(struct heap_node*) &handle->node.heap,
timer_less_than);
uv__handle_start(handle);
return 0;
}
int uv_timer_stop(uv_timer_t* handle) {
if (!uv__is_active(handle))
return 0;
static void timer_stop(uv_timer_t* handle) {
heap_remove(timer_heap(handle->loop),
(struct heap_node*) &handle->heap_node,
(struct heap_node*) &handle->node.heap,
timer_less_than);
uv__handle_stop(handle);
uv__queue_init(&handle->node.queue);
}
int uv_timer_stop(uv_timer_t* handle) {
if (uv__is_active(handle))
timer_stop(handle);
else
uv__queue_remove(&handle->node.queue);
return 0;
}
@ -148,7 +152,7 @@ int uv__next_timeout(const uv_loop_t* loop) {
if (heap_node == NULL)
return -1; /* block indefinitely */
handle = container_of(heap_node, uv_timer_t, heap_node);
handle = container_of(heap_node, uv_timer_t, node.heap);
if (handle->timeout <= loop->time)
return 0;
@ -163,17 +167,30 @@ int uv__next_timeout(const uv_loop_t* loop) {
void uv__run_timers(uv_loop_t* loop) {
struct heap_node* heap_node;
uv_timer_t* handle;
struct uv__queue* queue_node;
struct uv__queue ready_queue;
uv__queue_init(&ready_queue);
for (;;) {
heap_node = heap_min(timer_heap(loop));
if (heap_node == NULL)
break;
handle = container_of(heap_node, uv_timer_t, heap_node);
handle = container_of(heap_node, uv_timer_t, node.heap);
if (handle->timeout > loop->time)
break;
uv_timer_stop(handle);
timer_stop(handle);
uv__queue_insert_tail(&ready_queue, &handle->node.queue);
}
while (!uv__queue_empty(&ready_queue)) {
queue_node = uv__queue_head(&ready_queue);
uv__queue_remove(queue_node);
uv__queue_init(queue_node);
handle = container_of(queue_node, uv_timer_t, node.queue);
uv_timer_again(handle);
handle->timer_cb(handle);
}

2
test/test-list.h
View File

@ -227,6 +227,7 @@ TEST_DECLARE (timer_init)
TEST_DECLARE (timer_again)
TEST_DECLARE (timer_start_twice)
TEST_DECLARE (timer_order)
TEST_DECLARE (timer_zero_timeout)
TEST_DECLARE (timer_huge_timeout)
TEST_DECLARE (timer_huge_repeat)
TEST_DECLARE (timer_run_once)
@ -849,6 +850,7 @@ TASK_LIST_START
TEST_ENTRY (timer_again)
TEST_ENTRY (timer_start_twice)
TEST_ENTRY (timer_order)
TEST_ENTRY (timer_zero_timeout)
TEST_ENTRY (timer_huge_timeout)
TEST_ENTRY (timer_huge_repeat)
TEST_ENTRY (timer_run_once)

26
test/test-timer.c
View File

@ -31,6 +31,7 @@ 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 int zero_timeout_cb_calls = 0;
static uint64_t start_time;
static uv_timer_t tiny_timer;
static uv_timer_t huge_timer1;
@ -242,6 +243,31 @@ TEST_IMPL(timer_order) {
}
static void zero_timeout_cb(uv_timer_t* handle) {
ASSERT_OK(uv_timer_start(handle, zero_timeout_cb, 0, 0));
uv_stop(handle->loop);
zero_timeout_cb_calls++;
}
TEST_IMPL(timer_zero_timeout) {
uv_timer_t timer;
uv_loop_t* loop;
loop = uv_default_loop();
ASSERT_OK(uv_timer_init(loop, &timer));
ASSERT_OK(uv_timer_start(&timer, zero_timeout_cb, 0, 0));
ASSERT_EQ(1, uv_run(loop, UV_RUN_DEFAULT)); /* because of uv_stop() */
ASSERT_EQ(1, zero_timeout_cb_calls);
uv_close((uv_handle_t*) &timer, NULL);
ASSERT_OK(uv_run(loop, UV_RUN_DEFAULT));
ASSERT_EQ(1, zero_timeout_cb_calls);
MAKE_VALGRIND_HAPPY(loop);
return 0;
}
static void tiny_timer_cb(uv_timer_t* handle) {
ASSERT_PTR_EQ(handle, &tiny_timer);
uv_close((uv_handle_t*) &tiny_timer, NULL);