Skip to content

Commit d37995d

Browse files
committed
* Fix race condition in process event handling by using PID-based shared events
Problem: The async extension created separate event objects for each process monitoring request, leading to multiple waitpid() calls for the same PID. This caused race conditions where one waitpid() would succeed and subsequent calls would fail with ECHILD, resulting in "Cannot resume a coroutine that has not been suspended" errors. Solution: Modified process event system to use PID as hash key instead of event object address. Now libuv_new_process_event() returns existing event for a PID (with incremented ref count) or creates new one if none exists. This ensures only one waitpid() call per process. Changes: - libuv_new_process_event(): Added PID-based lookup and ref counting - libuv_handle_process_events(): Updated to use PID keys for verification - libuv_remove_process_event(): Modified to use PID keys and check ref count - libuv_add_process_event(): Simplified since hash insertion now handled by new_process_event Fixes intermittent failures in stream_select_remote_disconnect.phpt test.
1 parent 6d48411 commit d37995d

File tree

1 file changed

+36
-10
lines changed

1 file changed

+36
-10
lines changed

libuv_reactor.c

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -987,14 +987,17 @@ static void libuv_handle_process_events(void)
987987
for (i = 0; i < num_events; i++) {
988988
event = events_copy[i];
989989

990+
// Get PID to use as key for verification
991+
async_process_event_t *process = (async_process_event_t *) event;
992+
uintptr_t pid_key = (uintptr_t)process->event.process;
993+
990994
// Verify event is still in the HashTable (might have been removed)
991995
if (ASYNC_G(process_events) == NULL ||
992-
zend_hash_index_find_ptr(ASYNC_G(process_events), async_ptr_to_index(event)) == NULL) {
996+
zend_hash_index_find_ptr(ASYNC_G(process_events), pid_key) == NULL) {
993997
continue;
994998
}
995999

9961000
#ifndef PHP_WIN32
997-
async_process_event_t *process = (async_process_event_t *) event;
9981001
pid_t pid = (pid_t) process->event.process;
9991002
int status;
10001003
pid_t result = waitpid(pid, &status, WNOHANG);
@@ -1051,13 +1054,9 @@ static void libuv_add_process_event(zend_async_event_t *event)
10511054
return;
10521055
}
10531056

1054-
// Initialize process_events if needed
1055-
if (ASYNC_G(process_events) == NULL) {
1056-
ASYNC_G(process_events) = zend_new_array(0);
1057-
}
1058-
1059-
// Add event to the process events list (use pointer address as key)
1060-
zend_hash_index_add_ptr(ASYNC_G(process_events), async_ptr_to_index(event), event);
1057+
// Note: Event is already added to process_events hash table by
1058+
// libuv_get_or_create_process_event() using PID as key.
1059+
// This function now only ensures SIGCHLD handler is active.
10611060
}
10621061

10631062
/* }}} */
@@ -1069,7 +1068,14 @@ static void libuv_remove_process_event(zend_async_event_t *event)
10691068
return;
10701069
}
10711070

1072-
zend_hash_index_del(ASYNC_G(process_events), async_ptr_to_index(event));
1071+
// Get process handle from event to use as key
1072+
async_process_event_t *process_event = (async_process_event_t *)event;
1073+
uintptr_t pid_key = (uintptr_t)process_event->event.process;
1074+
1075+
// Check ref count before removing - only remove if this is the last reference
1076+
if (ZEND_ASYNC_EVENT_REFCOUNT(event) <= 1) {
1077+
zend_hash_index_del(ASYNC_G(process_events), pid_key);
1078+
}
10731079

10741080
// Only remove SIGCHLD handler if no more process events AND no regular signal events for SIGCHLD
10751081
if (zend_hash_num_elements(ASYNC_G(process_events)) == 0) {
@@ -1530,6 +1536,23 @@ zend_async_process_event_t *libuv_new_process_event(zend_process_t process_handl
15301536
{
15311537
START_REACTOR_OR_RETURN_NULL;
15321538

1539+
// Use process handle as key for hash lookup
1540+
uintptr_t pid_key = (uintptr_t)process_handle;
1541+
1542+
// Initialize process_events if needed
1543+
if (ASYNC_G(process_events) == NULL) {
1544+
ASYNC_G(process_events) = zend_new_array(0);
1545+
}
1546+
1547+
// Check if we already have an event for this process
1548+
zend_async_process_event_t *existing_event = zend_hash_index_find_ptr(ASYNC_G(process_events), pid_key);
1549+
if (existing_event != NULL) {
1550+
// Found existing event - increment ref count and return it
1551+
ZEND_ASYNC_EVENT_ADD_REF(&existing_event->base);
1552+
return existing_event;
1553+
}
1554+
1555+
// Create new event only if one doesn't exist
15331556
async_process_event_t *process_event = pecalloc(
15341557
1, extra_size != 0 ? sizeof(async_process_event_t) + extra_size : sizeof(async_process_event_t), 0);
15351558

@@ -1543,6 +1566,9 @@ zend_async_process_event_t *libuv_new_process_event(zend_process_t process_handl
15431566
process_event->event.base.stop = libuv_process_event_stop;
15441567
process_event->event.base.dispose = libuv_process_event_dispose;
15451568

1569+
// Add to hash table using PID as key
1570+
zend_hash_index_add_ptr(ASYNC_G(process_events), pid_key, &process_event->event);
1571+
15461572
return &process_event->event;
15471573
}
15481574

0 commit comments

Comments
 (0)