From be3f96fa5bee074862f878b316caeccfc7c2b190 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Tue, 11 Jul 2023 14:11:25 -0600 Subject: [PATCH] unix: match kqueue and epoll code Match the implementation for linux.c to kqueue.c in the code around the calls to kevent and epoll. In linux.c the code was made more DRY by moving the nfds check up (including a comment of why it's possible) and combining two if checks into one. In kqueue.c the EINTR check was moved to match linux.c and an assert that was added in 42cc412c has been moved to be called directly after the EINTR check. Since it should always be true regardless. Ref: https://github.com/libuv/libuv/pull/3893 Ref: https://github.com/nodejs/node/issues/48490 --- src/unix/kqueue.c | 19 +++++++++++-------- src/unix/linux.c | 41 +++++++++++------------------------------ 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/src/unix/kqueue.c b/src/unix/kqueue.c index b78242d3be4e..85183c88f1d3 100644 --- a/src/unix/kqueue.c +++ b/src/unix/kqueue.c @@ -260,9 +260,6 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { ARRAY_SIZE(events), timeout == -1 ? NULL : &spec); - if (nfds == -1) - assert(errno == EINTR); - if (pset != NULL) pthread_sigmask(SIG_UNBLOCK, pset, NULL); @@ -272,6 +269,12 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { */ uv__update_time(loop); + if (nfds == -1) + assert(errno == EINTR); + else if (nfds == 0) + /* Unlimited timeout should only return with events or signal. */ + assert(timeout != -1); + if (nfds == 0 || nfds == -1) { /* If kqueue is empty or interrupted, we might still have children ready * to reap immediately. */ @@ -285,13 +288,13 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { if (reset_timeout != 0) { timeout = user_timeout; reset_timeout = 0; - } else if (nfds == 0) { - /* Reached the user timeout value. */ - assert(timeout != -1); - return; } - /* Interrupted by a signal. Update timeout and poll again. */ + /* If nfds == 0 then we may have been inside the system call for longer + * than |timeout| milliseconds so we need to update the timestamp to + * avoid drift. + * If nfds == -1 then it was interrupted by a signal. Update timeout and + * poll again. */ goto update_timeout; } diff --git a/src/unix/linux.c b/src/unix/linux.c index 48b9c2c43e10..01af4c456ef3 100644 --- a/src/unix/linux.c +++ b/src/unix/linux.c @@ -1364,42 +1364,23 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { */ SAVE_ERRNO(uv__update_time(loop)); - if (nfds == 0) { + if (nfds == -1) + assert(errno == EINTR); + else if (nfds == 0) + /* Unlimited timeout should only return with events or signal. */ assert(timeout != -1); + if (nfds == 0 || nfds == -1) { if (reset_timeout != 0) { timeout = user_timeout; reset_timeout = 0; } - if (timeout == -1) - continue; - - if (timeout == 0) - break; - - /* We may have been inside the system call for longer than |timeout| - * milliseconds so we need to update the timestamp to avoid drift. - */ - goto update_timeout; - } - - if (nfds == -1) { - if (errno != EINTR) - abort(); - - if (reset_timeout != 0) { - timeout = user_timeout; - reset_timeout = 0; - } - - if (timeout == -1) - continue; - - if (timeout == 0) - break; - - /* Interrupted by a signal. Update timeout and poll again. */ + /* If nfds == 0 then we may have been inside the system call for longer + * than |timeout| milliseconds so we need to update the timestamp to + * avoid drift. + * If nfds == -1 then it was interrupted by a signal. Update timeout and + * poll again. */ goto update_timeout; } @@ -1509,13 +1490,13 @@ void uv__io_poll(uv_loop_t* loop, int timeout) { break; } +update_timeout: if (timeout == 0) break; if (timeout == -1) continue; -update_timeout: assert(timeout > 0); real_timeout -= (loop->time - base);