Skip to content

Commit f521165

Browse files
committed
#9: * Fixed a memory leak issue in Scope + onFinally.
1 parent 6fae1be commit f521165

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

coroutine.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,10 +409,10 @@ static void finally_handlers_iterator_dtor(zend_async_iterator_t *zend_iterator)
409409
efree(context);
410410
iterator->extended_data = NULL;
411411

412-
if (ZEND_ASYNC_EVENT_REF(&scope->scope.event) > 1) {
412+
if (ZEND_ASYNC_EVENT_REF(&scope->scope.event) > 0) {
413413
ZEND_ASYNC_EVENT_DEL_REF(&scope->scope.event);
414414

415-
if (ZEND_ASYNC_EVENT_REF(&scope->scope.event) == 1) {
415+
if (ZEND_ASYNC_EVENT_REF(&scope->scope.event) <= 1) {
416416
scope->scope.try_to_dispose(&scope->scope);
417417
}
418418
}
@@ -464,7 +464,14 @@ bool async_call_finally_handlers(HashTable *finally_handlers, finally_handlers_c
464464
iterator->extended_data = context;
465465
iterator->extended_dtor = finally_handlers_iterator_dtor;
466466
async_iterator_run_in_coroutine(iterator, priority);
467-
ZEND_ASYNC_EVENT_ADD_REF(&child_scope->event);
467+
468+
//
469+
// We retain ownership of the Scope in order to be able to handle exceptions from the Finally handlers.
470+
// example: finally_handlers_iterator_dtor
471+
// If the onFinally handlers throw an exception, it will end up in the Scope,
472+
// so it’s important that the Scope is not destroyed before that moment.
473+
//
474+
ZEND_ASYNC_EVENT_ADD_REF(&context->scope->event);
468475

469476
if (UNEXPECTED(EG(exception))) {
470477
return false;

scope.c

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -990,26 +990,29 @@ static bool scope_try_to_dispose(zend_async_scope_t *scope)
990990
{
991991
async_scope_t *async_scope = (async_scope_t *) scope;
992992

993-
if (ZEND_ASYNC_SCOPE_IS_DISPOSED(scope)) {
993+
if (ZEND_ASYNC_SCOPE_IS_DISPOSING(scope)) {
994994
return true;
995995
}
996996

997997
if (false == SCOPE_CAN_BE_DISPOSED(async_scope)) {
998998
return false;
999999
}
10001000

1001-
ZEND_ASYNC_SCOPE_SET_DISPOSED(scope);
1001+
ZEND_ASYNC_SCOPE_SET_DISPOSING(scope);
10021002

10031003
// Dispose all child scopes
10041004
for (uint32_t i = 0; i < async_scope->scope.scopes.length; ++i) {
10051005
async_scope_t *child_scope = (async_scope_t *) async_scope->scope.scopes.data[i];
10061006
child_scope->scope.event.dispose(&child_scope->scope.event);
10071007
if (UNEXPECTED(EG(exception))) {
1008+
ZEND_ASYNC_SCOPE_CLR_DISPOSING(scope);
10081009
// If an exception occurs during child scope disposal, we stop further processing
10091010
return false;
10101011
}
10111012
}
10121013

1014+
ZEND_ASYNC_SCOPE_CLR_DISPOSING(scope);
1015+
10131016
// Dispose the scope
10141017
async_scope->scope.event.dispose(&async_scope->scope.event);
10151018
return true;
@@ -1532,12 +1535,6 @@ static zend_always_inline bool try_to_handle_exception(
15321535

15331536
static void async_scope_call_finally_handlers_dtor(finally_handlers_context_t *context)
15341537
{
1535-
zend_async_scope_t *scope = context->target;
1536-
if (ZEND_ASYNC_EVENT_REF(&scope->event) > 0) {
1537-
ZEND_ASYNC_EVENT_DEL_REF(&scope->event);
1538-
}
1539-
1540-
scope->try_to_dispose(scope);
15411538
context->target = NULL;
15421539
}
15431540

@@ -1566,7 +1563,6 @@ static bool async_scope_call_finally_handlers(async_scope_t *scope)
15661563
zend_array_destroy(finally_handlers);
15671564
return false;
15681565
} else {
1569-
ZEND_ASYNC_EVENT_ADD_REF(&scope->scope.event);
15701566
return true;
15711567
}
15721568
}

0 commit comments

Comments
 (0)