#9: * fix scope_try_to_dispose Fixed cyclic Scope release from children due to missing reentrancy handling.

* Fixed an issue with the set_previous_exception function caused by an extra reference deletion.

Author: EdmondDantes

coroutine.c

@@ -379,12 +379,11 @@ static void finally_handlers_iterator_dtor(zend_async_iterator_t *zend_iterator)
 	}
 
 	finally_handlers_context_t *context = iterator->extended_data;
+	async_scope_t *scope = (async_scope_t *) context->scope;
+	context->scope = NULL;
 
 	// Throw CompositeException if any exceptions were collected
 	if (context->composite_exception != NULL) {
-
-		async_scope_t *scope = (async_scope_t *) context->scope;
-
 		if (ZEND_ASYNC_SCOPE_CATCH(
 			&scope->scope,
 			&context->coroutine->coroutine,
@@ -394,13 +393,13 @@ static void finally_handlers_iterator_dtor(zend_async_iterator_t *zend_iterator)
 			ZEND_ASYNC_SCOPE_IS_DISPOSE_SAFELY(&scope->scope)
 		)) {
 			OBJ_RELEASE(context->composite_exception);
-		} else {
-			async_rethrow_exception(context->composite_exception);
+			context->composite_exception = NULL;
 		}
-
-		context->composite_exception = NULL;
 	}
 
+	zend_object * composite_exception = context->composite_exception;
+	context->composite_exception = NULL;
+
 	if (context->dtor != NULL) {
 		context->dtor(context);
 		context->dtor = NULL;
@@ -410,6 +409,18 @@ static void finally_handlers_iterator_dtor(zend_async_iterator_t *zend_iterator)
 	efree(context);
 	iterator->extended_data = NULL;
 
+	if (ZEND_ASYNC_EVENT_REF(&scope->scope.event) > 1) {
+		ZEND_ASYNC_EVENT_DEL_REF(&scope->scope.event);
+
+		if (ZEND_ASYNC_EVENT_REF(&scope->scope.event) == 1) {
+			scope->scope.try_to_dispose(&scope->scope);
+		}
+	}
+
+	if (composite_exception != NULL) {
+		async_rethrow_exception(composite_exception);
+	}
+
 	//
 	// If everything is correct,
 	// the Scope will destroy itself as soon as the coroutine created within it completes execution.
@@ -453,6 +464,7 @@ bool async_call_finally_handlers(HashTable *finally_handlers, finally_handlers_c
 	iterator->extended_data = context;
 	iterator->extended_dtor = finally_handlers_iterator_dtor;
 	async_iterator_run_in_coroutine(iterator, priority);
+	ZEND_ASYNC_EVENT_ADD_REF(&child_scope->event);
 
 	if (UNEXPECTED(EG(exception))) {
 		return false;

scheduler.c

@@ -471,7 +471,6 @@ void start_graceful_shutdown(void)
 
 	if (UNEXPECTED(EG(exception) != NULL)) {
 		zend_exception_set_previous(EG(exception), ZEND_ASYNC_EXIT_EXCEPTION);
-		GC_DELREF(ZEND_ASYNC_EXIT_EXCEPTION);
 		ZEND_ASYNC_EXIT_EXCEPTION = EG(exception);
 		GC_ADDREF(EG(exception));
 		zend_clear_exception();
@@ -484,7 +483,6 @@ static void finally_shutdown(void)
 {
 	if (ZEND_ASYNC_EXIT_EXCEPTION != NULL && EG(exception) != NULL) {
 		zend_exception_set_previous(EG(exception), ZEND_ASYNC_EXIT_EXCEPTION);
-		GC_DELREF(ZEND_ASYNC_EXIT_EXCEPTION);
 		ZEND_ASYNC_EXIT_EXCEPTION = EG(exception);
 		GC_ADDREF(EG(exception));
 		zend_clear_exception();
@@ -498,7 +496,6 @@ static void finally_shutdown(void)
 	if (UNEXPECTED(EG(exception))) {
 		if (ZEND_ASYNC_EXIT_EXCEPTION != NULL) {
 			zend_exception_set_previous(EG(exception), ZEND_ASYNC_EXIT_EXCEPTION);
-			GC_DELREF(ZEND_ASYNC_EXIT_EXCEPTION);
 			ZEND_ASYNC_EXIT_EXCEPTION = EG(exception);
 			GC_ADDREF(EG(exception));
 		}
@@ -709,7 +706,6 @@ void async_scheduler_main_coroutine_suspend(void)
 	//
 	if (EG(exception) != NULL && exit_exception != NULL) {
 		zend_exception_set_previous(EG(exception), exit_exception);
-		GC_DELREF(exit_exception);
 	} else if (exit_exception != NULL) {
 		async_rethrow_exception(exit_exception);
 	}
@@ -855,7 +851,6 @@ void async_scheduler_coroutine_suspend(zend_fiber_transfer *transfer)
 
 		if (ZEND_ASYNC_EXIT_EXCEPTION != NULL) {
 			zend_exception_set_previous(exception, ZEND_ASYNC_EXIT_EXCEPTION);
-			GC_DELREF(ZEND_ASYNC_EXIT_EXCEPTION);
 			ZEND_ASYNC_EXIT_EXCEPTION = exception;
 		} else {
 			ZEND_ASYNC_EXIT_EXCEPTION = exception;
@@ -982,7 +977,6 @@ void async_scheduler_main_loop(void)
 
 	if (EG(exception) != NULL && exit_exception != NULL) {
 		zend_exception_set_previous(EG(exception), exit_exception);
-		GC_DELREF(exit_exception);
 		exit_exception = EG(exception);
 		GC_ADDREF(exit_exception);
 		zend_clear_exception();

scope.c

@@ -990,10 +990,16 @@ static bool scope_try_to_dispose(zend_async_scope_t *scope)
 {
 	async_scope_t *async_scope = (async_scope_t *) scope;
 
+	if (ZEND_ASYNC_SCOPE_IS_DISPOSED(scope)) {
+		return true;
+	}
+
 	if (false == SCOPE_CAN_BE_DISPOSED(async_scope)) {
 		return false;
 	}
 
+	ZEND_ASYNC_SCOPE_SET_DISPOSED(scope);
+
 	// Dispose all child scopes
 	for (uint32_t i = 0; i < async_scope->scope.scopes.length; ++i) {
 		async_scope_t *child_scope = (async_scope_t *) async_scope->scope.scopes.data[i];
@@ -1111,6 +1117,7 @@ static void scope_dispose(zend_async_event_t *scope_event)
 	}
 
 	if (scope->scope.scope_object != NULL) {
+		fprintf(stderr, "unlink %p\n", scope->scope.scope_object);
 		((async_scope_object_t *) scope->scope.scope_object)->scope = NULL;
 		scope->scope.scope_object = NULL;
 	}
@@ -1154,6 +1161,7 @@ zend_async_scope_t * async_new_scope(zend_async_scope_t * parent_scope, const bo
 
 	if (with_zend_object) {
 		scope_object = ZEND_OBJECT_ALLOC_EX(sizeof(async_scope_object_t), async_ce_scope);
+		fprintf(stderr, "regular new scope_object %p\n", scope_object);
 
 		zend_object_std_init(&scope_object->std, async_ce_scope);
 		object_properties_init(&scope_object->std, async_ce_scope);
@@ -1235,6 +1243,8 @@ static void scope_destroy(zend_object *object)
 	scope_object->scope = NULL;
 	scope->scope.scope_object = NULL;
 
+	fprintf(stderr, "scope_destroy %p\n", object);
+
 	// At this point, the user-defined Scope object is about to be destroyed.
 	// This means we are obligated to cancel the Scope and all its child Scopes along with their coroutines.
 	// However, the Scope itself will not be destroyed.
@@ -1436,6 +1446,7 @@ static zend_always_inline bool try_to_handle_exception(
 		// The PHP Scope object might already be destroyed by the time the internal Scope still exists.
 		// To normalize this situation, we’ll create a fake Scope object that will serve as a bridge.
 		scope_object = ZEND_OBJECT_ALLOC_EX(sizeof(async_scope_object_t), async_ce_scope);
+		fprintf(stderr, "new scope_object %p\n", scope_object);
 		zend_object_std_init(&scope_object->std, async_ce_scope);
 		object_properties_init(&scope_object->std, async_ce_scope);
 

tests/scope/034-scope_cancellation_double_error.phpt

@@ -35,14 +35,6 @@ $child_coroutine = $child_scope->spawn(function() {
 
     $coroutine = \Async\currentCoroutine();
     some_function();
-
-    $coroutine->onFinally(function() {
-        echo "child finally handler executed\n";
-    });
-
-    suspend();
-    echo "child should not complete\n";
-    return "child_result";
 });
 
 echo "coroutines with finally handlers spawned\n";