Skip to content

Commit 9e1b5b5

Browse files
EdmondDantesEdmondDantes
committed
Fix exception handling and memory management in async API
 • Add ZEND_ASYNC_EVENT_F_EXCEPTION_HANDLED flag for tracking processed exceptions • Fix zend_async_waker_callback_resolve to automatically capture exceptions • Refactor exception handling macros to work at event level instead of coroutine level • Fix double-free memory bug in reactor module • Optimize exception propagation between coroutines Changes: - Move exception handling logic from coroutine level to event level - Automatically mark exceptions as handled when passed to another coroutine - Prevent further propagation of captured exceptions - Unify API for working with event flags - Resolve memory corruption issues in reactor cleanup
1 parent 3f14c26 commit 9e1b5b5

File tree

4 files changed

+32
-46
lines changed

4 files changed

+32
-46
lines changed

‎async.c‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -321,18 +321,20 @@ PHP_FUNCTION(Async_awaitFirstSuccess)
321321
HashTable * return_array = zend_new_array(2);
322322

323323
zval val;
324+
ZVAL_NULL(&val);
324325

325-
if (zend_hash_num_elements(results) == 0) {
326-
ZVAL_NULL(&val);
327-
} else {
328-
ZVAL_COPY(&val, zend_hash_index_find(results, 0));
329-
}
326+
ZEND_HASH_FOREACH_VAL(results, zval *item) {
327+
ZVAL_COPY(&val, item);
328+
break;
329+
} ZEND_HASH_FOREACH_END();
330330

331331
zend_hash_next_index_insert_new(return_array, &val);
332332

333333
ZVAL_ARR(&val, errors);
334334
zend_hash_next_index_insert_new(return_array, &val);
335335

336+
zend_array_release(results);
337+
336338
RETURN_ARR(return_array);
337339
}
338340

‎async_API.c‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,17 @@ void async_waiting_callback(
429429
return;
430430
}
431431

432-
if (await_context->results != NULL && ZEND_ASYNC_EVENT_WILL_ZVAL_RESULT(event) && result != NULL) {
432+
// If the exception exists, and we are ignoring errors, we do not resume the coroutine.
433+
if (exception != NULL && await_context->ignore_errors) {
434+
// But if there's no one left to wait for, stop waiting.
435+
if (await_context->total != 0 && await_context->resolved_count >= await_context->total) {
436+
ZEND_ASYNC_RESUME(await_callback->callback.coroutine);
437+
}
438+
439+
return;
440+
}
441+
442+
if (exception == NULL && await_context->results != NULL && ZEND_ASYNC_EVENT_WILL_ZVAL_RESULT(event) && result != NULL) {
433443

434444
const zval *success = NULL;
435445

@@ -445,7 +455,7 @@ void async_waiting_callback(
445455
}
446456

447457
if (success != NULL) {
448-
zval_add_ref(result);
458+
Z_TRY_ADDREF_P(result);
449459
}
450460
}
451461

‎libuv_reactor.c‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,6 @@ static void libuv_poll_dispose(zend_async_event_t *event)
243243
async_poll_event_t *poll = (async_poll_event_t *)(event);
244244

245245
uv_close((uv_handle_t *)&poll->uv_handle, libuv_close_handle_cb);
246-
247-
pefree(event, 0);
248246
}
249247
/* }}} */
250248

@@ -289,6 +287,7 @@ zend_async_poll_event_t* libuv_new_poll_event(
289287
poll->uv_handle.data = poll;
290288
poll->event.events = events;
291289
poll->event.base.extra_offset = sizeof(async_poll_event_t);
290+
poll->event.base.ref_count = 1;
292291

293292
// Initialize the event methods
294293
poll->event.base.add_callback = libuv_add_callback;
@@ -390,8 +389,6 @@ static void libuv_timer_dispose(zend_async_event_t *event)
390389
async_timer_event_t *timer = (async_timer_event_t *)(event);
391390

392391
uv_close((uv_handle_t *)&timer->uv_handle, libuv_close_handle_cb);
393-
394-
pefree(event, 0);
395392
}
396393
/* }}} */
397394

@@ -416,6 +413,7 @@ zend_async_timer_event_t* libuv_new_timer_event(const zend_ulong timeout, const
416413
event->event.timeout = timeout;
417414
event->event.is_periodic = is_periodic;
418415
event->event.base.extra_offset = sizeof(async_timer_event_t);
416+
event->event.base.ref_count = 1;
419417

420418
event->event.base.add_callback = libuv_add_callback;
421419
event->event.base.del_callback = libuv_remove_callback;
@@ -502,8 +500,6 @@ static void libuv_signal_dispose(zend_async_event_t *event)
502500
async_signal_event_t *signal = (async_signal_event_t *)(event);
503501

504502
uv_close((uv_handle_t *)&signal->uv_handle, libuv_close_handle_cb);
505-
506-
pefree(event, 0);
507503
}
508504
/* }}} */
509505

@@ -527,6 +523,7 @@ zend_async_signal_event_t* libuv_new_signal_event(int signum, size_t extra_size)
527523
signal->uv_handle.data = signal;
528524
signal->event.signal = signum;
529525
signal->event.base.extra_offset = sizeof(async_signal_event_t);
526+
signal->event.base.ref_count = 1;
530527

531528
signal->event.base.add_callback = libuv_add_callback;
532529
signal->event.base.del_callback = libuv_remove_callback;
@@ -828,8 +825,6 @@ static void libuv_process_event_dispose(zend_async_event_t *event)
828825
process->hJob = NULL;
829826
}
830827
#endif
831-
832-
pefree(event, 0);
833828
}
834829
/* }}} */
835830

@@ -844,6 +839,7 @@ zend_async_process_event_t * libuv_new_process_event(zend_process_t process_hand
844839

845840
process_event->event.process = process_handle;
846841
process_event->event.base.extra_offset = sizeof(async_process_event_t);
842+
process_event->event.base.ref_count = 1;
847843

848844
process_event->event.base.add_callback = libuv_add_callback;
849845
process_event->event.base.del_callback = libuv_remove_callback;
@@ -976,8 +972,6 @@ static void libuv_filesystem_dispose(zend_async_event_t *event)
976972
}
977973

978974
uv_close((uv_handle_t *)&fs_event->uv_handle, libuv_close_handle_cb);
979-
980-
pefree(event, 0);
981975
}
982976
/* }}} */
983977

@@ -1002,6 +996,7 @@ zend_async_filesystem_event_t* libuv_new_filesystem_event(zend_string * path, co
1002996
fs_event->event.path = zend_string_copy(path);
1003997
fs_event->event.flags = flags;
1004998
fs_event->event.base.extra_offset = sizeof(async_filesystem_event_t);
999+
fs_event->event.base.ref_count = 1;
10051000

10061001
fs_event->event.base.add_callback = libuv_add_callback;
10071002
fs_event->event.base.del_callback = libuv_remove_callback;
@@ -1087,8 +1082,6 @@ static void libuv_dns_nameinfo_dispose(zend_async_event_t *event)
10871082
async_dns_nameinfo_t *name_info = (async_dns_nameinfo_t *)(event);
10881083

10891084
uv_close((uv_handle_t *)&name_info->uv_handle, libuv_close_handle_cb);
1090-
1091-
pefree(event, 0);
10921085
}
10931086
/* }}} */
10941087

@@ -1113,6 +1106,7 @@ static zend_async_dns_nameinfo_t * libuv_getnameinfo(const struct sockaddr *addr
11131106

11141107
name_info->uv_handle.data = name_info;
11151108
name_info->event.base.extra_offset = sizeof(async_dns_nameinfo_t);
1109+
name_info->event.base.ref_count = 1;
11161110

11171111
name_info->event.base.add_callback = libuv_add_callback;
11181112
name_info->event.base.del_callback = libuv_remove_callback;
@@ -1184,8 +1178,6 @@ static void libuv_dns_getaddrinfo_dispose(zend_async_event_t *event)
11841178
async_dns_addrinfo_t *addr_info = (async_dns_addrinfo_t *)(event);
11851179

11861180
uv_close((uv_handle_t *)&addr_info->uv_handle, libuv_close_handle_cb);
1187-
1188-
pefree(event, 0);
11891181
}
11901182
/* }}} */
11911183

@@ -1212,6 +1204,7 @@ static zend_async_dns_addrinfo_t* libuv_getaddrinfo(
12121204

12131205
addr_info->uv_handle.data = addr_info;
12141206
addr_info->event.base.extra_offset = sizeof(async_dns_nameinfo_t);
1207+
addr_info->event.base.ref_count = 1;
12151208

12161209
addr_info->event.base.add_callback = libuv_add_callback;
12171210
addr_info->event.base.del_callback = libuv_remove_callback;
@@ -1454,8 +1447,6 @@ static void libuv_exec_dispose(zend_async_event_t *event)
14541447
exec->quoted_cmd = NULL;
14551448
}
14561449
#endif
1457-
1458-
pefree(event, 0);
14591450
}
14601451
/* }}} */
14611452

@@ -1549,6 +1540,8 @@ static zend_async_exec_event_t * libuv_new_exec_event(
15491540

15501541
ZEND_ASYNC_INCREASE_EVENT_COUNT;
15511542

1543+
exec->event.base.ref_count = 1;
1544+
15521545
exec->event.base.add_callback = libuv_add_callback;
15531546
exec->event.base.del_callback = libuv_remove_callback;
15541547
exec->event.base.start = libuv_exec_start;

‎tests/await/008-awaitFirstSuccess_basic.phpt‎

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ echo "start\n";
1111

1212
$coroutines = [
1313
spawn(function() {
14-
delay(50);
14+
delay(10);
1515
throw new RuntimeException("first error");
1616
}),
1717
spawn(function() {
@@ -20,7 +20,7 @@ $coroutines = [
2020
}),
2121
spawn(function() {
2222
delay(30);
23-
throw new RuntimeException("second error");
23+
return "another success";
2424
}),
2525
];
2626

@@ -29,13 +29,13 @@ var_dump($result);
2929

3030
echo "end\n";
3131
?>
32-
--EXPECT--
32+
--EXPECTF--
3333
start
3434
array(2) {
3535
[0]=>
3636
string(7) "success"
3737
[1]=>
38-
array(2) {
38+
array(1) {
3939
[0]=>
4040
object(RuntimeException)#%d (7) {
4141
["message":protected]=>
@@ -55,25 +55,6 @@ array(2) {
5555
["previous":"Exception":private]=>
5656
NULL
5757
}
58-
[2]=>
59-
object(RuntimeException)#%d (7) {
60-
["message":protected]=>
61-
string(12) "second error"
62-
["string":"Exception":private]=>
63-
string(0) ""
64-
["code":protected]=>
65-
int(0)
66-
["file":protected]=>
67-
string(%d) "%s"
68-
["line":protected]=>
69-
int(%d)
70-
["trace":"Exception":private]=>
71-
array(%d) {
72-
%a
73-
}
74-
["previous":"Exception":private]=>
75-
NULL
76-
}
7758
}
7859
}
7960
end

0 commit comments

Comments
 (0)