Skip to content

Commit bde6fce

Browse files
committed
* Fixed process waiting logic for UNIX-like systems
1 parent 391404f commit bde6fce

File tree

2 files changed

+39
-22
lines changed

2 files changed

+39
-22
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919
- The add/dispose methods correctly increment the counter
2020
- Memory leaks fixed
2121
- Fixed await iterator logic for `awaitXXX` functions
22+
- Fixed process waiting logic for UNIX-like systems
2223

2324
### Changed
2425
- **Memory Optimization**: Enhanced memory allocation for async structures

libuv_reactor.c

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ static void libuv_remove_signal_event(int signum, zend_async_event_t *event);
3737
static void libuv_add_process_event(zend_async_event_t *event);
3838
static void libuv_remove_process_event(zend_async_event_t *event);
3939
static void libuv_handle_process_events(void);
40-
static void libuv_handle_signal_events(int signum);
40+
static void libuv_handle_signal_events(const int signum);
4141
static void libuv_signal_close_cb(uv_handle_t *handle);
4242

4343
// Forward declarations for cleanup functions
@@ -828,6 +828,23 @@ zend_async_signal_event_t *libuv_new_signal_event(int signum, size_t extra_size)
828828
/////////////////////////////////////////////////////////////////////////////////
829829
/// Global Signal Management API
830830
/////////////////////////////////////////////////////////////////////////////////
831+
/**
832+
* **UNIX Signal Handling**
833+
*
834+
* The Unix signal handling code allows creating multiple wait events for the same signal, but in reality,
835+
* there will be only one actual handler for the EventLoop.
836+
*
837+
* > Note that the `SIGCHLD` handler is also used to detect the process-event.
838+
*
839+
* **Implementation details of process handling.**
840+
*
841+
* Because the process exit code can only be retrieved once,
842+
* while multiple coroutines may want to wait for the same process,
843+
* we use a single EventLoop event for each unique process ID.
844+
*
845+
* Thus, ASYNC_G(process_events) is a hash table with the key as ProcessId
846+
* and the value as an Event for process handling.
847+
**/
831848

832849
/* {{{ libuv_signal_close_cb */
833850
static void libuv_signal_close_cb(uv_handle_t *handle)
@@ -869,20 +886,26 @@ static uv_signal_t *libuv_get_or_create_signal_handler(int signum)
869886
handler = pecalloc(1, sizeof(uv_signal_t), 0);
870887

871888
int error = uv_signal_init(UVLOOP, handler);
872-
if (error < 0) {
889+
if (UNEXPECTED(error < 0)) {
873890
async_throw_error("Failed to initialize signal handle: %s", uv_strerror(error));
874891
pefree(handler, 0);
875892
return NULL;
876893
}
877894

878895
error = uv_signal_start(handler, libuv_global_signal_callback, signum);
879-
if (error < 0) {
896+
if (UNEXPECTED(error < 0)) {
880897
async_throw_error("Failed to start signal handle: %s", uv_strerror(error));
881898
uv_close((uv_handle_t *) handler, libuv_signal_close_cb);
882899
return NULL;
883900
}
884901

885-
zend_hash_index_add_ptr(ASYNC_G(signal_handlers), signum, handler);
902+
if (UNEXPECTED(zend_hash_index_add_ptr(ASYNC_G(signal_handlers), signum, handler) == NULL)) {
903+
async_throw_error("Failed to store signal handler");
904+
uv_signal_stop(handler);
905+
uv_close((uv_handle_t *) handler, libuv_signal_close_cb);
906+
return NULL;
907+
}
908+
886909
return handler;
887910
}
888911

@@ -906,11 +929,13 @@ static void libuv_add_signal_event(int signum, zend_async_event_t *event)
906929
HashTable *events_list = zend_hash_index_find_ptr(ASYNC_G(signal_events), signum);
907930
if (events_list == NULL) {
908931
events_list = zend_new_array(0);
909-
zend_hash_index_add_ptr(ASYNC_G(signal_events), signum, events_list);
910932
}
911933

912934
// Add event to the list (use pointer address as key)
913-
zend_hash_index_add_ptr(events_list, async_ptr_to_index(event), event);
935+
if (UNEXPECTED(zend_hash_index_add_ptr(events_list, async_ptr_to_index(event), event) == NULL)) {
936+
async_throw_error("Failed to store signal event");
937+
return;
938+
}
914939
}
915940

916941
/* }}} */
@@ -1024,7 +1049,7 @@ static void libuv_handle_process_events(void)
10241049
/* }}} */
10251050

10261051
/* {{{ libuv_handle_signal_events */
1027-
static void libuv_handle_signal_events(int signum)
1052+
static void libuv_handle_signal_events(const int signum)
10281053
{
10291054
if (ASYNC_G(signal_events) == NULL) {
10301055
return;
@@ -1049,14 +1074,7 @@ static void libuv_handle_signal_events(int signum)
10491074
static void libuv_add_process_event(zend_async_event_t *event)
10501075
{
10511076
// Ensure SIGCHLD handler exists
1052-
uv_signal_t *handler = libuv_get_or_create_signal_handler(SIGCHLD);
1053-
if (handler == NULL) {
1054-
return;
1055-
}
1056-
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.
1077+
libuv_get_or_create_signal_handler(SIGCHLD);
10601078
}
10611079

10621080
/* }}} */
@@ -1070,12 +1088,8 @@ static void libuv_remove_process_event(zend_async_event_t *event)
10701088

10711089
// Get process handle from event to use as key
10721090
async_process_event_t *process_event = (async_process_event_t *)event;
1073-
uintptr_t pid_key = (uintptr_t)process_event->event.process;
10741091

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-
}
1092+
zend_hash_index_del(ASYNC_G(process_events), (uintptr_t)process_event->event.process);
10791093

10801094
// Only remove SIGCHLD handler if no more process events AND no regular signal events for SIGCHLD
10811095
if (zend_hash_num_elements(ASYNC_G(process_events)) == 0) {
@@ -1566,8 +1580,10 @@ zend_async_process_event_t *libuv_new_process_event(zend_process_t process_handl
15661580
process_event->event.base.stop = libuv_process_event_stop;
15671581
process_event->event.base.dispose = libuv_process_event_dispose;
15681582

1569-
// Add to hash table using PID as key
1570-
zend_hash_index_add_ptr(ASYNC_G(process_events), pid_key, &process_event->event);
1583+
if (UNEXPECTED(zend_hash_index_add_ptr(ASYNC_G(process_events), pid_key, &process_event->event) == NULL)) {
1584+
async_throw_error("Failed to store process event");
1585+
return NULL;
1586+
}
15711587

15721588
return &process_event->event;
15731589
}

0 commit comments

Comments
 (0)