Skip to content

Commit

Permalink
darwin,process: feed kevent the signal to reap children (libuv#3893)
Browse files Browse the repository at this point in the history
Since we are emulating this event, but are not using the pending_queue,
we need to make sure it is accounted for properly. Also DRY some of the
reset_timeout code here.

This was observed to cause a hang in certain rare cases, particularly on
M1 machines.

Do a bit of code cleanup too, since we do not need to initialize the
internal signal handling pipe if it will not be used.
  • Loading branch information
vtjnash authored Feb 1, 2023
1 parent f1b4c76 commit 42cc412
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 29 deletions.
1 change: 1 addition & 0 deletions src/unix/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ static int uv__backend_timeout(const uv_loop_t* loop) {
(uv__has_active_handles(loop) || uv__has_active_reqs(loop)) &&
QUEUE_EMPTY(&loop->pending_queue) &&
QUEUE_EMPTY(&loop->idle_handles) &&
(loop->flags & UV_LOOP_REAP_CHILDREN) == 0 &&
loop->closing_handles == NULL)
return uv__next_timeout(loop);
return 0;
Expand Down
1 change: 1 addition & 0 deletions src/unix/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ int uv__kqueue_init(uv_loop_t* loop);
int uv__platform_loop_init(uv_loop_t* loop);
void uv__platform_loop_delete(uv_loop_t* loop);
void uv__platform_invalidate_fd(uv_loop_t* loop, int fd);
int uv__process_init(uv_loop_t* loop);

/* various */
void uv__async_close(uv_async_t* handle);
Expand Down
43 changes: 18 additions & 25 deletions src/unix/kqueue.c
Original file line number Diff line number Diff line change
Expand Up @@ -250,43 +250,36 @@ 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);

/* 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) {
if (reset_timeout != 0) {
timeout = user_timeout;
reset_timeout = 0;
if (timeout == -1)
continue;
if (timeout > 0)
goto update_timeout;
uv__update_time(loop);

if (nfds == 0 || nfds == -1) {
/* If kqueue is empty or interrupted, we might still have children ready
* to reap immediately. */
if (loop->flags & UV_LOOP_REAP_CHILDREN) {
loop->flags &= ~UV_LOOP_REAP_CHILDREN;
uv__wait_children(loop);
assert((reset_timeout == 0 ? timeout : user_timeout) == 0);
return; /* Equivalent to fall-through behavior. */
}

assert(timeout != -1);
return;
}

if (nfds == -1) {
if (errno != EINTR)
abort();

if (reset_timeout != 0) {
timeout = user_timeout;
reset_timeout = 0;
}

if (timeout == 0)
} else if (nfds == 0) {
/* Reached the user timeout value. */
assert(timeout != -1);
return;

if (timeout == -1)
continue;
}

/* Interrupted by a signal. Update timeout and poll again. */
goto update_timeout;
Expand Down Expand Up @@ -413,13 +406,13 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
return;
}

update_timeout:
if (timeout == 0)
return;

if (timeout == -1)
continue;

update_timeout:
assert(timeout > 0);

diff = loop->time - base;
Expand Down
5 changes: 1 addition & 4 deletions src/unix/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,9 @@ int uv_loop_init(uv_loop_t* loop) {
goto fail_platform_init;

uv__signal_global_once_init();
err = uv_signal_init(loop, &loop->child_watcher);
err = uv__process_init(loop);
if (err)
goto fail_signal_init;

uv__handle_unref(&loop->child_watcher);
loop->child_watcher.flags |= UV_HANDLE_INTERNAL;
QUEUE_INIT(&loop->process_handles);

err = uv_rwlock_init(&loop->cloexec_lock);
Expand Down
27 changes: 27 additions & 0 deletions src/unix/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,28 @@ static void uv__chld(uv_signal_t* handle, int signum) {
assert(signum == SIGCHLD);
uv__wait_children(handle->loop);
}


int uv__process_init(uv_loop_t* loop) {
int err;

err = uv_signal_init(loop, &loop->child_watcher);
if (err)
return err;
uv__handle_unref(&loop->child_watcher);
loop->child_watcher.flags |= UV_HANDLE_INTERNAL;
return 0;
}


#else
int uv__process_init(uv_loop_t* loop) {
memset(&loop->child_watcher, 0, sizeof(loop->child_watcher));
return 0;
}
#endif


void uv__wait_children(uv_loop_t* loop) {
uv_process_t* process;
int exit_status;
Expand All @@ -105,6 +125,7 @@ void uv__wait_children(uv_loop_t* loop) {
continue;
options = 0;
process->flags &= ~UV_HANDLE_REAP;
loop->nfds--;
#else
options = WNOHANG;
#endif
Expand Down Expand Up @@ -1012,6 +1033,10 @@ int uv_spawn(uv_loop_t* loop,
process->flags |= UV_HANDLE_REAP;
loop->flags |= UV_LOOP_REAP_CHILDREN;
}
/* This prevents uv__io_poll() from bailing out prematurely, being unaware
* that we added an event here for it to react to. We will decrement this
* again after the waitpid call succeeds. */
loop->nfds++;
#endif

process->pid = pid;
Expand Down Expand Up @@ -1080,6 +1105,8 @@ int uv_kill(int pid, int signum) {
void uv__process_close(uv_process_t* handle) {
QUEUE_REMOVE(&handle->queue);
uv__handle_stop(handle);
#ifdef UV_USE_SIGCHLD
if (QUEUE_EMPTY(&handle->loop->process_handles))
uv_signal_stop(&handle->loop->child_watcher);
#endif
}
2 changes: 2 additions & 0 deletions src/unix/signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ static int uv__signal_loop_once_init(uv_loop_t* loop) {


int uv__signal_loop_fork(uv_loop_t* loop) {
if (loop->signal_pipefd[0] == -1)
return 0;
uv__io_stop(loop, &loop->signal_io_watcher, POLLIN);
uv__close(loop->signal_pipefd[0]);
uv__close(loop->signal_pipefd[1]);
Expand Down

0 comments on commit 42cc412

Please sign in to comment.