* Coroutine tests fixed.

  Bug in active coroutine counter state fixed.
  Coroutine finalization code optimized.

Author: EdmondDantes

coroutine.c

@@ -520,112 +520,119 @@ void async_coroutine_finalize(zend_fiber_transfer *transfer, async_coroutine_t *
 		ZEND_COROUTINE_FINISH(&coroutine->coroutine);
 	}
 
-	/* Cleanup switch handlers */
-	zend_coroutine_switch_handlers_destroy(&coroutine->coroutine);
+	bool do_bailout = false;
 
-	// call coroutines handlers
-	zend_object * exception = NULL;
+	zend_try {
 
-	if (EG(exception)) {
-		if (EG(prev_exception)) {
-			zend_exception_set_previous(EG(exception), EG(prev_exception));
-			EG(prev_exception) = NULL;
-		}
+		/* Cleanup switch handlers */
+		zend_coroutine_switch_handlers_destroy(&coroutine->coroutine);
 
-		exception = EG(exception);
-		GC_ADDREF(exception);
+		// call coroutines handlers
+		zend_object * exception = NULL;
 
-		zend_clear_exception();
+		if (EG(exception)) {
+			if (EG(prev_exception)) {
+				zend_exception_set_previous(EG(exception), EG(prev_exception));
+				EG(prev_exception) = NULL;
+			}
 
-		if (zend_is_graceful_exit(exception) || zend_is_unwind_exit(exception)) {
-			OBJ_RELEASE(exception);
-			exception = NULL;
-		}
-	}
+			exception = EG(exception);
+			GC_ADDREF(exception);
 
-	// Hold the exception inside coroutine if it is not NULL.
-	if (exception != NULL) {
-		if (coroutine->coroutine.exception != NULL) {
-			// If the coroutine already has an exception, we do not overwrite it.
-			// This is to prevent losing the original exception in case of multiple exceptions.
-			zend_exception_set_previous(exception, coroutine->coroutine.exception);
+			zend_clear_exception();
+
+			if (zend_is_graceful_exit(exception) || zend_is_unwind_exit(exception)) {
+				OBJ_RELEASE(exception);
+				exception = NULL;
+			}
 		}
 
-		coroutine->coroutine.exception = exception;
-		GC_ADDREF(exception);
-	} else if (coroutine->coroutine.exception != NULL) {
-		// If the coroutine has an exception, we keep it.
-		exception = coroutine->coroutine.exception;
-		GC_ADDREF(exception);
-	}
+		// Hold the exception inside coroutine if it is not NULL.
+		if (exception != NULL) {
+			if (coroutine->coroutine.exception != NULL) {
+				// If the coroutine already has an exception, we do not overwrite it.
+				// This is to prevent losing the original exception in case of multiple exceptions.
+				zend_exception_set_previous(exception, coroutine->coroutine.exception);
+			}
 
-	zend_exception_save();
-	// Mark second parameter of zend_async_callbacks_notify as ZVAL
-	ZEND_ASYNC_EVENT_SET_ZVAL_RESULT(&coroutine->coroutine.event);
-	ZEND_COROUTINE_CLR_EXCEPTION_HANDLED(&coroutine->coroutine);
-	ZEND_ASYNC_CALLBACKS_NOTIFY(&coroutine->coroutine.event, &coroutine->coroutine.result, exception);
-	zend_async_callbacks_free(&coroutine->coroutine.event);
+			coroutine->coroutine.exception = exception;
+			GC_ADDREF(exception);
+		} else if (coroutine->coroutine.exception != NULL) {
+			// If the coroutine has an exception, we keep it.
+			exception = coroutine->coroutine.exception;
+			GC_ADDREF(exception);
+		}
 
-	if (coroutine->coroutine.internal_context != NULL) {
-		zend_async_coroutine_internal_context_dispose(&coroutine->coroutine);
-	}
+		zend_exception_save();
+		// Mark second parameter of zend_async_callbacks_notify as ZVAL
+		ZEND_ASYNC_EVENT_SET_ZVAL_RESULT(&coroutine->coroutine.event);
+		ZEND_COROUTINE_CLR_EXCEPTION_HANDLED(&coroutine->coroutine);
+		ZEND_ASYNC_CALLBACKS_NOTIFY(&coroutine->coroutine.event, &coroutine->coroutine.result, exception);
+		zend_async_callbacks_free(&coroutine->coroutine.event);
+
+		if (coroutine->coroutine.internal_context != NULL) {
+			zend_async_coroutine_internal_context_dispose(&coroutine->coroutine);
+		}
 
-	// Call finally handlers if any
-	if (coroutine->finally_handlers != NULL && zend_hash_num_elements(coroutine->finally_handlers) > 0) {
-		coroutine_call_finally_handlers(coroutine);
-	}
+		// Call finally handlers if any
+		if (coroutine->finally_handlers != NULL && zend_hash_num_elements(coroutine->finally_handlers) > 0) {
+			coroutine_call_finally_handlers(coroutine);
+		}
 
-	zend_async_waker_destroy(&coroutine->coroutine);
+		zend_async_waker_destroy(&coroutine->coroutine);
 
-	zend_exception_restore();
+		zend_exception_restore();
 
-	// If the exception was handled by any handler, we do not propagate it further.
-	// Cancellation-type exceptions are considered handled in all cases and are not propagated further.
-	if (exception != NULL
-		&& (ZEND_COROUTINE_IS_EXCEPTION_HANDLED(&coroutine->coroutine)
-			|| instanceof_function(exception->ce, zend_ce_cancellation_exception))) {
-		OBJ_RELEASE(exception);
-		exception = NULL;
-	}
+		// If the exception was handled by any handler, we do not propagate it further.
+		// Cancellation-type exceptions are considered handled in all cases and are not propagated further.
+		if (exception != NULL
+			&& (ZEND_COROUTINE_IS_EXCEPTION_HANDLED(&coroutine->coroutine)
+				|| instanceof_function(exception->ce, zend_ce_cancellation_exception))) {
+			OBJ_RELEASE(exception);
+			exception = NULL;
+		}
 
-	// Before the exception leads to graceful termination,
-	// we give one last chance to handle it using Scope handlers.
-	if (exception != NULL && ZEND_ASYNC_SCOPE_CATCH(
-			coroutine->coroutine.scope,
-			&coroutine->coroutine,
-			NULL,
-			exception,
-			false,
-			ZEND_ASYNC_SCOPE_IS_DISPOSE_SAFELY(coroutine->coroutine.scope)
-			)) {
-		OBJ_RELEASE(exception);
-		exception = NULL;
-	}
+		// Before the exception leads to graceful termination,
+		// we give one last chance to handle it using Scope handlers.
+		if (exception != NULL && ZEND_ASYNC_SCOPE_CATCH(
+				coroutine->coroutine.scope,
+				&coroutine->coroutine,
+				NULL,
+				exception,
+				false,
+				ZEND_ASYNC_SCOPE_IS_DISPOSE_SAFELY(coroutine->coroutine.scope)
+				)) {
+			OBJ_RELEASE(exception);
+			exception = NULL;
+		}
 
-	// Notify the async scope that the coroutine has finished.
-	// For the Scheduler, the coroutine's Scope may be undefined.
-	if (EXPECTED(coroutine->coroutine.scope != NULL)) {
-		async_scope_notify_coroutine_finished(coroutine);
-		coroutine->coroutine.scope = NULL;
-	}
+		// Notify the async scope that the coroutine has finished.
+		// For the Scheduler, the coroutine's Scope may be undefined.
+		if (EXPECTED(coroutine->coroutine.scope != NULL)) {
+			async_scope_notify_coroutine_finished(coroutine);
+			coroutine->coroutine.scope = NULL;
+		}
 
-	// Otherwise, we rethrow the exception.
-	if (exception != NULL) {
-		zend_throw_exception_internal(exception);
-	}
+		// Otherwise, we rethrow the exception.
+		if (exception != NULL) {
+			zend_throw_exception_internal(exception);
+		}
 
-	if (EG(exception)) {
-		if (!(coroutine->flags & ZEND_FIBER_FLAG_DESTROYED)
-			|| !(zend_is_graceful_exit(EG(exception)) || zend_is_unwind_exit(EG(exception)))
-		) {
-			coroutine->flags |= ZEND_FIBER_FLAG_THREW;
-			transfer->flags = ZEND_FIBER_TRANSFER_FLAG_ERROR;
+		if (EG(exception)) {
+			if (!(coroutine->flags & ZEND_FIBER_FLAG_DESTROYED)
+				|| !(zend_is_graceful_exit(EG(exception)) || zend_is_unwind_exit(EG(exception)))
+			) {
+				coroutine->flags |= ZEND_FIBER_FLAG_THREW;
+				transfer->flags = ZEND_FIBER_TRANSFER_FLAG_ERROR;
 
-			ZVAL_OBJ_COPY(&transfer->value, EG(exception));
-		}
+				ZVAL_OBJ_COPY(&transfer->value, EG(exception));
+			}
 
-		zend_clear_exception();
-	}
+			zend_clear_exception();
+		}
+	} zend_catch {
+		do_bailout = true;
+	} zend_end_try();
 
 	if (EXPECTED(ZEND_ASYNC_SCHEDULER != &coroutine->coroutine)) {
 		// Permanently remove the coroutine from the Scheduler.
@@ -639,6 +646,10 @@ void async_coroutine_finalize(zend_fiber_transfer *transfer, async_coroutine_t *
 			ZEND_ASYNC_DECREASE_COROUTINE_COUNT
 		}
 	}
+
+	if (UNEXPECTED(do_bailout)) {
+		zend_bailout();
+	}
 }
 
 /**
@@ -701,7 +712,7 @@ ZEND_STACK_ALIGNED void async_coroutine_execute(zend_fiber_transfer *transfer)
 	ZEND_COROUTINE_SET_STARTED(&coroutine->coroutine);
 
 	/* Call switch handlers for coroutine entering */
-	if (UNEXPECTED(&coroutine->coroutine.switch_handlers != NULL)) {
+	if (UNEXPECTED(coroutine->coroutine.switch_handlers != NULL)) {
 		ZEND_COROUTINE_ENTER(&coroutine->coroutine);
 	}
 
@@ -753,18 +764,6 @@ ZEND_STACK_ALIGNED void async_coroutine_execute(zend_fiber_transfer *transfer)
 	} zend_catch {
 		coroutine->flags |= ZEND_FIBER_FLAG_BAILOUT;
 		transfer->flags = ZEND_FIBER_TRANSFER_FLAG_BAILOUT;
-
-		if (EXPECTED(ZEND_ASYNC_SCHEDULER != &coroutine->coroutine)) {
-			// Permanently remove the coroutine from the Scheduler.
-			if (UNEXPECTED(zend_hash_index_del(&ASYNC_G(coroutines), coroutine->std.handle) == FAILURE)) {
-				async_throw_error("Failed to remove coroutine from the list");
-			}
-
-			// Decrease the active coroutine count if the coroutine is not a zombie.
-			if (false == ZEND_COROUTINE_IS_ZOMBIE(&coroutine->coroutine)) {
-				ZEND_ASYNC_DECREASE_COROUTINE_COUNT
-			}
-		}
 	} zend_end_try();
 
 	coroutine->context.cleanup = &async_coroutine_cleanup;
@@ -1014,6 +1013,7 @@ void async_coroutine_cancel(zend_coroutine_t *zend_coroutine, zend_object *error
 		// In any other case, the cancellation exception overrides the existing exception.
 		//
 		ZEND_ASYNC_WAKER_APPLY_CANCELLATION(waker, error, transfer_error);
+		ZEND_ASYNC_DECREASE_COROUTINE_COUNT;
 		async_scheduler_coroutine_enqueue(zend_coroutine);
 		return;
 	}

scheduler.c

@@ -259,8 +259,22 @@ static bool resolve_deadlocks(void)
 {
 	zval *value;
 
+	const zend_long active_coroutines = ZEND_ASYNC_ACTIVE_COROUTINE_COUNT;
+	const zend_long real_coroutines = zend_hash_num_elements(&ASYNC_G(coroutines));
+
+	if (active_coroutines > real_coroutines) {
+		async_warning(
+			"The active coroutine counter contains an incorrect value: %u, real counter: %u.",
+			active_coroutines, real_coroutines
+		);
+	}
+
+	if (real_coroutines == 0) {
+		return false;
+	}
+
 	async_warning(
-		"no active coroutines, deadlock detected. Coroutines in waiting: %u", ZEND_ASYNC_ACTIVE_COROUTINE_COUNT
+		"no active coroutines, deadlock detected. Coroutines in waiting: %u", real_coroutines
 	);
 
 	ZEND_HASH_FOREACH_VAL(&ASYNC_G(coroutines), value)
@@ -655,6 +669,7 @@ void async_scheduler_main_coroutine_suspend(void)
 	switch_to_scheduler(NULL);
 
 	ZEND_ASYNC_CURRENT_COROUTINE = NULL;
+	ZEND_ASSERT(ZEND_ASYNC_ACTIVE_COROUTINE_COUNT == 0 && "The active coroutine counter must be 1 at this point");
 	ZEND_ASYNC_DEACTIVATE;
 
 	if (ASYNC_G(main_transfer)) {

scope.c

@@ -104,7 +104,7 @@ static bool scope_can_be_disposed(async_scope_t *scope, bool with_zombies, bool
 static void scope_check_completion_and_notify(async_scope_t *scope, bool with_zombies);
 
 // Finally handlers execution functions
-static void async_scope_call_finally_handlers(async_scope_t *scope);
+static bool async_scope_call_finally_handlers(async_scope_t *scope);
 
 // Structure for scope finally handlers context
 typedef struct {
@@ -1068,18 +1068,21 @@ static void scope_dispose(zend_async_event_t *scope_event)
 		return;
 	}
 
-	ZEND_ASYNC_EVENT_DEL_REF(scope_event);
+	if (ZEND_ASYNC_EVENT_REF(scope_event) == 1) {
+		ZEND_ASYNC_EVENT_DEL_REF(scope_event);
+	}
 
 	async_scope_t *scope = (async_scope_t *) scope_event;
 
 	ZEND_ASSERT(scope->coroutines.length == 0 && scope->scope.scopes.length == 0
 		&& "Scope should be empty before disposal");
 
-	if (scope->finally_handlers != NULL && zend_hash_num_elements(scope->finally_handlers) > 0) {
-		async_scope_call_finally_handlers(scope);
-		if (ZEND_ASYNC_EVENT_REF(scope_event) == 1) {
-			return;
-		}
+	if (scope->finally_handlers != NULL
+		&& zend_hash_num_elements(scope->finally_handlers) > 0
+		&& async_scope_call_finally_handlers(scope)) {
+		// If finally handlers were called, we don't dispose the scope yet
+		ZEND_ASYNC_EVENT_ADD_REF(&scope->scope.event);
+		return;
 	}
 
 	if (scope->scope.parent_scope) {
@@ -1106,9 +1109,13 @@ static void scope_dispose(zend_async_event_t *scope_event)
 	// Free exception handlers
 	if (scope->exception_fci != NULL || scope->exception_fcc != NULL) {
 		zend_free_fci(scope->exception_fci, scope->exception_fcc);
+		scope->exception_fci = NULL;
+		scope->exception_fcc = NULL;
 	}
 	if (scope->child_exception_fci != NULL || scope->child_exception_fcc != NULL) {
 		zend_free_fci(scope->child_exception_fci, scope->child_exception_fcc);
+		scope->child_exception_fci = NULL;
+		scope->child_exception_fcc = NULL;
 	}
 
 	// Free finally handlers
@@ -1379,18 +1386,25 @@ static zend_always_inline bool try_to_handle_exception(
 	return false; // Exception not handled
 }
 
-static void async_scope_call_finally_handlers(async_scope_t *scope)
+static void async_scope_call_finally_handlers_dtor(finally_handlers_context_t *context)
+{
+	zend_async_scope_t *scope = context->target;
+	scope->try_to_dispose(scope);
+	context->target = NULL;
+}
+
+static bool async_scope_call_finally_handlers(async_scope_t *scope)
 {
 	if (scope->finally_handlers == NULL || zend_hash_num_elements(scope->finally_handlers) == 0) {
-		return;
+		return false;
 	}
 
 	HashTable *finally_handlers = scope->finally_handlers;
 	scope->finally_handlers = NULL;
 	finally_handlers_context_t *finally_context = ecalloc(1, sizeof(finally_handlers_context_t));
 	finally_context->target = scope;
 	finally_context->scope = &scope->scope;
-	finally_context->dtor = NULL;
+	finally_context->dtor = async_scope_call_finally_handlers_dtor;
 	finally_context->params_count = 1;
 
 	if (scope->scope.scope_object != NULL) {
@@ -1402,5 +1416,9 @@ static void async_scope_call_finally_handlers(async_scope_t *scope)
 	if (false == async_call_finally_handlers(finally_handlers, finally_context, 1)) {
 		efree(finally_context);
 		zend_array_destroy(finally_handlers);
+		return false;
+	} else {
+		ZEND_ASYNC_EVENT_ADD_REF(&scope->scope.event);
+		return true;
 	}
 }