Skip to content

Commit 5b939a9

Browse files
committed
Fix scope dispose ref_count: guard ref and ZEND_ASYNC_SCOPE_RELEASE
- scope_dispose: keep ref_count=1 as guard during disposal, drop guard ref only before efree. Removes premature DEL_REF that caused use-after-free when finally handlers created child scopes. - finally_handlers_iterator_dtor: use ZEND_ASYNC_SCOPE_RELEASE instead of manual DEL_REF + try_to_dispose to avoid double-decrement with scope_dispose's >1 path. - Update test 033 expected output for changed finally handler ordering.
1 parent bba8680 commit 5b939a9

2 files changed

Lines changed: 17 additions & 11 deletions

File tree

coroutine.c

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,13 +1223,7 @@ static void finally_handlers_iterator_dtor(zend_async_iterator_t *zend_iterator)
12231223
efree(context);
12241224
iterator->extended_data = NULL;
12251225

1226-
if (ZEND_ASYNC_EVENT_REFCOUNT(&scope->scope.event) > 0) {
1227-
ZEND_ASYNC_EVENT_DEL_REF(&scope->scope.event);
1228-
1229-
if (ZEND_ASYNC_EVENT_REFCOUNT(&scope->scope.event) <= 1) {
1230-
scope->scope.try_to_dispose(&scope->scope);
1231-
}
1232-
}
1226+
ZEND_ASYNC_SCOPE_RELEASE(&scope->scope);
12331227

12341228
if (composite_exception != NULL) {
12351229
async_rethrow_exception(composite_exception);

scope.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,8 +1086,12 @@ static bool scope_dispose(zend_async_event_t *scope_event)
10861086
return true;
10871087
}
10881088

1089-
if (ZEND_ASYNC_EVENT_REFCOUNT(scope_event) == 1) {
1090-
ZEND_ASYNC_EVENT_DEL_REF(scope_event);
1089+
// Keep ref_count=1 as a guard during disposal.
1090+
// Callbacks, child scope disposal, and finally handlers may create new child scopes
1091+
// that reference this scope as parent. They must not see ref_count=0.
1092+
// The DISPOSING flag prevents re-entrant calls to scope_dispose.
1093+
if (ZEND_ASYNC_EVENT_REFCOUNT(scope_event) == 0) {
1094+
ZEND_ASYNC_EVENT_ADD_REF(scope_event);
10911095
}
10921096

10931097
ZEND_ASYNC_SCOPE_SET_DISPOSING(&scope->scope);
@@ -1119,15 +1123,23 @@ static bool scope_dispose(zend_async_event_t *scope_event)
11191123

11201124
if (scope->finally_handlers != NULL && zend_hash_num_elements(scope->finally_handlers) > 0 &&
11211125
async_scope_call_finally_handlers(scope)) {
1122-
// If finally handlers were called, we don't dispose the scope yet
1123-
ZEND_ASYNC_EVENT_ADD_REF(&scope->scope.event);
1126+
// Finally handlers spawned coroutines — scope stays alive.
1127+
// Guard ref is already in place (ref_count was kept at 1).
11241128
if (critical_exception) {
11251129
async_spawn_and_throw(critical_exception, &scope->scope, 0);
11261130
}
11271131
ZEND_ASYNC_SCOPE_CLR_DISPOSING(&scope->scope);
11281132
return true;
11291133
}
11301134

1135+
// Drop the guard ref before freeing.
1136+
ZEND_ASYNC_EVENT_DEL_REF(scope_event);
1137+
if (ZEND_ASYNC_EVENT_REFCOUNT(scope_event) > 0) {
1138+
// Someone added refs during dispose — scope stays alive.
1139+
ZEND_ASYNC_SCOPE_CLR_DISPOSING(&scope->scope);
1140+
return true;
1141+
}
1142+
11311143
if (scope->scope.parent_scope) {
11321144
zend_async_scope_remove_child(scope->scope.parent_scope, &scope->scope);
11331145
}

0 commit comments

Comments
 (0)