From b992fb117ec9d68976f118b327ac391a3d4b2495 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:51:14 +0000 Subject: [PATCH 1/2] #216 Release the SIGCHLD watch when the last process event goes libuv_process_event_dispose() deleted its entry from process_events directly, while the teardown lives in libuv_remove_process_event(). An event released while already stopped therefore emptied the table and left the handler armed: nothing raises SIGCHLD again, yet the handler pins the loop, so a worker with no work left never exits. The teardown is now its own function, called from both paths. A hidden timer had the same effect for a different reason. HIDDEN keeps a pool healthcheck out of active_event_count, but uv_timer_start leaves the handle referenced and uv_run does not return. Hidden timers now drop that reference, as the thread-notify handles already did by hand. --- libuv_reactor.c | 92 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 26 deletions(-) diff --git a/libuv_reactor.c b/libuv_reactor.c index 774ee4c..4bd6024 100644 --- a/libuv_reactor.c +++ b/libuv_reactor.c @@ -1083,6 +1083,14 @@ static bool libuv_timer_start(zend_async_event_t *event) return false; } + // A hidden timer is a background heartbeat — a connection pool healthcheck, + // say. Keeping it out of active_event_count is not enough: the uv handle + // still holds a loop reference, so uv_run never returns and the process + // hangs with no work left to do. + if (ZEND_ASYNC_EVENT_IS_HIDDEN(event)) { + uv_unref((uv_handle_t *) &timer->uv_handle); + } + event->loop_ref_count++; ZEND_ASYNC_INCREASE_EVENT_COUNT(event); return true; @@ -1655,6 +1663,10 @@ static void libuv_handle_process_events(void) ZEND_ASYNC_CALLBACKS_NOTIFY(event, NULL, NULL); // Process event will be removed when stopped } + /* ECHILD is deliberately not treated as "settle this event": a notified + * event stays in the table until its waiter resumes, so a second sweep + * before that sees ECHILD for the child this sweep just reaped, and + * settling again would overwrite the exit code with a made-up one. */ #endif } @@ -1694,6 +1706,58 @@ static void libuv_add_process_event(zend_async_event_t *event) /* }}} */ +/* {{{ libuv_release_process_watch + * + * Drops the SIGCHLD machinery once the last process event is gone. + * + * Called from every path that empties the table, not just the stop path: an + * event released while already stopped is deleted straight from the hash, and + * without this the handler outlives the last watcher. Nothing raises it again, + * yet it holds the reactor loop, so the process runs out of work and still + * refuses to exit. */ +static void libuv_release_process_watch(void) +{ + if (ASYNC_G(process_events) == NULL || zend_hash_num_elements(ASYNC_G(process_events)) > 0) { + return; + } + + bool has_sigchld_signal_events = false; + + if (ASYNC_G(signal_events) != NULL) { + HashTable *sigchld_events = zend_hash_index_find_ptr(ASYNC_G(signal_events), SIGCHLD); + if (sigchld_events != NULL && zend_hash_num_elements(sigchld_events) > 0) { + has_sigchld_signal_events = true; + } + } + + // A plain signal() subscriber on SIGCHLD owns the handler for as long as it waits. + if (!has_sigchld_signal_events && ASYNC_G(signal_handlers) != NULL) { + uv_signal_t *handler = zend_hash_index_find_ptr(ASYNC_G(signal_handlers), SIGCHLD); + if (handler != NULL) { + if ((bool) (uintptr_t) handler->data) { + /* A Zend-chain subscriber (pcntl etc.) still needs delivery — + * keep the handler armed but stop pinning the loop, exactly as + * libuv_remove_signal_event does. Closing it here would revert + * the OS disposition and leave that subscriber permanently deaf. */ + uv_unref((uv_handle_t *) handler); + } else { + uv_signal_stop(handler); +#ifdef ZEND_SIGNALS + libuv_restore_signal_handler(SIGCHLD); +#endif + uv_close((uv_handle_t *) handler, libuv_signal_close_cb); + zend_hash_index_del(ASYNC_G(signal_handlers), SIGCHLD); + } + } + } + + zend_hash_destroy(ASYNC_G(process_events)); + pefree(ASYNC_G(process_events), 0); + ASYNC_G(process_events) = NULL; +} + +/* }}} */ + /* {{{ libuv_remove_process_event */ static void libuv_remove_process_event(zend_async_event_t *event) { @@ -1706,32 +1770,7 @@ static void libuv_remove_process_event(zend_async_event_t *event) zend_hash_index_del(ASYNC_G(process_events), (uintptr_t) process_event->event.process); - // Only remove SIGCHLD handler if no more process events AND no regular signal events for SIGCHLD - if (zend_hash_num_elements(ASYNC_G(process_events)) == 0) { - bool has_sigchld_signal_events = false; - - // Check if there are regular signal events for SIGCHLD - if (ASYNC_G(signal_events) != NULL) { - HashTable *sigchld_events = zend_hash_index_find_ptr(ASYNC_G(signal_events), SIGCHLD); - if (sigchld_events != NULL && zend_hash_num_elements(sigchld_events) > 0) { - has_sigchld_signal_events = true; - } - } - - // Only remove handler if no signal events exist for SIGCHLD - if (!has_sigchld_signal_events && ASYNC_G(signal_handlers) != NULL) { - uv_signal_t *handler = zend_hash_index_find_ptr(ASYNC_G(signal_handlers), SIGCHLD); - if (handler != NULL) { - uv_signal_stop(handler); - uv_close((uv_handle_t *) handler, libuv_signal_close_cb); - zend_hash_index_del(ASYNC_G(signal_handlers), SIGCHLD); - } - } - - zend_hash_destroy(ASYNC_G(process_events)); - pefree(ASYNC_G(process_events), 0); - ASYNC_G(process_events) = NULL; - } + libuv_release_process_watch(); } /* }}} */ @@ -2170,6 +2209,7 @@ static bool libuv_process_event_dispose(zend_async_event_t *event) const zend_process_t proc_handle = ((zend_async_process_event_t *) event)->process; if ((uintptr_t) proc_handle != 0 && ASYNC_G(process_events) != NULL) { zend_hash_index_del(ASYNC_G(process_events), (zend_ulong) (uintptr_t) proc_handle); + libuv_release_process_watch(); } #ifdef PHP_WIN32 From 6ff1d294f4ca98ad7e78360cb5fa2f203fbb6876 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Tue, 4 Aug 2026 07:21:09 +0000 Subject: [PATCH 2/2] #216 Trim the comments, keep the two fixes Restores the two comments the extraction displaced, and cuts the rest to what the code does not already say. --- libuv_reactor.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/libuv_reactor.c b/libuv_reactor.c index 4bd6024..4743e90 100644 --- a/libuv_reactor.c +++ b/libuv_reactor.c @@ -1083,10 +1083,8 @@ static bool libuv_timer_start(zend_async_event_t *event) return false; } - // A hidden timer is a background heartbeat — a connection pool healthcheck, - // say. Keeping it out of active_event_count is not enough: the uv handle - // still holds a loop reference, so uv_run never returns and the process - // hangs with no work left to do. + // Hidden means "keeps firing, but is not a reason to stay alive": unref drops + // the timer out of uv_loop_alive() without stopping it. if (ZEND_ASYNC_EVENT_IS_HIDDEN(event)) { uv_unref((uv_handle_t *) &timer->uv_handle); } @@ -1663,10 +1661,8 @@ static void libuv_handle_process_events(void) ZEND_ASYNC_CALLBACKS_NOTIFY(event, NULL, NULL); // Process event will be removed when stopped } - /* ECHILD is deliberately not treated as "settle this event": a notified - * event stays in the table until its waiter resumes, so a second sweep - * before that sees ECHILD for the child this sweep just reaped, and - * settling again would overwrite the exit code with a made-up one. */ + // ECHILD is not settled here: a notified event outlives this sweep, so a + // second sweep would overwrite its exit code with a made-up one. #endif } @@ -1707,14 +1703,8 @@ static void libuv_add_process_event(zend_async_event_t *event) /* }}} */ /* {{{ libuv_release_process_watch - * - * Drops the SIGCHLD machinery once the last process event is gone. - * - * Called from every path that empties the table, not just the stop path: an - * event released while already stopped is deleted straight from the hash, and - * without this the handler outlives the last watcher. Nothing raises it again, - * yet it holds the reactor loop, so the process runs out of work and still - * refuses to exit. */ + * Drops the SIGCHLD handler once the last process event is gone. Call from every + * path that empties the table. */ static void libuv_release_process_watch(void) { if (ASYNC_G(process_events) == NULL || zend_hash_num_elements(ASYNC_G(process_events)) > 0) { @@ -1723,6 +1713,7 @@ static void libuv_release_process_watch(void) bool has_sigchld_signal_events = false; + // Check if there are regular signal events for SIGCHLD if (ASYNC_G(signal_events) != NULL) { HashTable *sigchld_events = zend_hash_index_find_ptr(ASYNC_G(signal_events), SIGCHLD); if (sigchld_events != NULL && zend_hash_num_elements(sigchld_events) > 0) { @@ -1730,15 +1721,12 @@ static void libuv_release_process_watch(void) } } - // A plain signal() subscriber on SIGCHLD owns the handler for as long as it waits. + // Only remove handler if no signal events exist for SIGCHLD if (!has_sigchld_signal_events && ASYNC_G(signal_handlers) != NULL) { uv_signal_t *handler = zend_hash_index_find_ptr(ASYNC_G(signal_handlers), SIGCHLD); if (handler != NULL) { if ((bool) (uintptr_t) handler->data) { - /* A Zend-chain subscriber (pcntl etc.) still needs delivery — - * keep the handler armed but stop pinning the loop, exactly as - * libuv_remove_signal_event does. Closing it here would revert - * the OS disposition and leave that subscriber permanently deaf. */ + // pcntl still needs delivery; closing reverts the OS disposition. uv_unref((uv_handle_t *) handler); } else { uv_signal_stop(handler);