Merge pull request #10 from true-async/6-proper-handling-of-internal-references-to-zvals-for-coroutines-and-scopes

6 proper handling of internal references to zvals for coroutines and scopes

Author: EdmondDantes

CHANGELOG.md

@@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.3.0] - TBD
+
+### Added
+- **Garbage Collection Support**: Implemented comprehensive GC handlers for async objects
+    - Added `async_coroutine_object_gc()` function to track all ZVALs in coroutine structures
+    - Added `async_scope_object_gc()` function to track ZVALs in scope structures  
+    - Proper GC tracking for context HashTables (values and keys)
+    - GC support for finally handlers, exception handlers, and function call parameters
+    - GC tracking for waker events, internal context, and nested async structures
+    - Prevents memory leaks in complex async applications with circular references
+
+### Fixed
+- Memory management improvements for long-running async applications
+- Proper cleanup of coroutine and scope objects during garbage collection cycles
+
 ## [0.2.0] - TBD
 
 ### Added

coroutine.c

@@ -25,6 +25,7 @@
 #include "scope.h"
 #include "zend_common.h"
 #include "zend_exceptions.h"
+#include "zend_generators.h"
 #include "zend_ini.h"
 
 #define METHOD(name) PHP_METHOD(Async_Coroutine, name)
@@ -1182,6 +1183,138 @@ zend_coroutine_t *async_new_coroutine(zend_async_scope_t *scope)
 	return &coroutine->coroutine;
 }
 
+static HashTable *async_coroutine_object_gc(zend_object *object, zval **table, int *num)
+{
+	async_coroutine_t *coroutine = (async_coroutine_t *)ZEND_ASYNC_OBJECT_TO_EVENT(object);
+	zend_get_gc_buffer *buf = zend_get_gc_buffer_create();
+
+	/* Always add basic ZVALs from coroutine structure */
+	zend_get_gc_buffer_add_zval(buf, &coroutine->coroutine.result);
+	
+	/* Add objects that may be present */
+	if (coroutine->coroutine.exception) {
+		zend_get_gc_buffer_add_obj(buf, coroutine->coroutine.exception);
+	}
+	
+	if (coroutine->deferred_cancellation) {
+		zend_get_gc_buffer_add_obj(buf, coroutine->deferred_cancellation);
+	}
+
+	/* Add finally handlers if present */
+	if (coroutine->finally_handlers) {
+		zval *val;
+		ZEND_HASH_FOREACH_VAL(coroutine->finally_handlers, val) {
+			zend_get_gc_buffer_add_zval(buf, val);
+		} ZEND_HASH_FOREACH_END();
+	}
+
+	/* Add internal context HashTable if present */
+	if (coroutine->coroutine.internal_context) {
+		zval *val;
+		ZEND_HASH_FOREACH_VAL(coroutine->coroutine.internal_context, val) {
+			zend_get_gc_buffer_add_zval(buf, val);
+		} ZEND_HASH_FOREACH_END();
+	}
+
+	/* Add fcall function name and parameters if present */
+	if (coroutine->coroutine.fcall) {
+		zend_get_gc_buffer_add_zval(buf, &coroutine->coroutine.fcall->fci.function_name);
+		
+		/* Add function parameters */
+		if (coroutine->coroutine.fcall->fci.param_count > 0 && coroutine->coroutine.fcall->fci.params) {
+			for (uint32_t i = 0; i < coroutine->coroutine.fcall->fci.param_count; i++) {
+				zend_get_gc_buffer_add_zval(buf, &coroutine->coroutine.fcall->fci.params[i]);
+			}
+		}
+	}
+
+	/* Add waker-related ZVALs if present */
+	if (coroutine->coroutine.waker) {
+		zend_get_gc_buffer_add_zval(buf, &coroutine->coroutine.waker->result);
+		
+		if (coroutine->coroutine.waker->error) {
+			zend_get_gc_buffer_add_obj(buf, coroutine->coroutine.waker->error);
+		}
+		
+		/* Add events HashTable contents */
+		zval *event_val;
+		zval zval_object;
+		ZEND_HASH_FOREACH_VAL(&coroutine->coroutine.waker->events, event_val) {
+
+			zend_async_event_t *event = (zend_async_event_t *) Z_PTR_P(event_val);
+
+			if (ZEND_ASYNC_EVENT_IS_REFERENCE(event) || ZEND_ASYNC_EVENT_IS_ZEND_OBJ(event)) {
+				ZVAL_OBJ(&zval_object, ZEND_ASYNC_EVENT_TO_OBJECT(event));
+				zend_get_gc_buffer_add_zval(buf, &zval_object);
+			}
+		} ZEND_HASH_FOREACH_END();
+		
+		/* Add triggered events if present */
+		if (coroutine->coroutine.waker->triggered_events) {
+			ZEND_HASH_FOREACH_VAL(coroutine->coroutine.waker->triggered_events, event_val) {
+				zend_get_gc_buffer_add_zval(buf, event_val);
+			} ZEND_HASH_FOREACH_END();
+		}
+	}
+
+	/* Add context ZVALs if present */
+	if (coroutine->coroutine.context) {
+		/* Cast to actual context implementation to access HashTables */
+		async_context_t *context = (async_context_t *)coroutine->coroutine.context;
+		
+		/* Add all values from context->values HashTable */
+		zval *val;
+		ZEND_HASH_FOREACH_VAL(&context->values, val) {
+			zend_get_gc_buffer_add_zval(buf, val);
+		} ZEND_HASH_FOREACH_END();
+		
+		/* Add all object keys from context->keys HashTable */
+		ZEND_HASH_FOREACH_VAL(&context->keys, val) {
+			zend_get_gc_buffer_add_zval(buf, val);
+		} ZEND_HASH_FOREACH_END();
+	}
+
+	/* Check if we should traverse execution stack (similar to fibers) */
+	if (coroutine->context.status != ZEND_FIBER_STATUS_SUSPENDED || 
+		!coroutine->execute_data) {
+		zend_get_gc_buffer_use(buf, table, num);
+		return NULL;
+	}
+
+	/* Traverse execution stack for suspended coroutines */
+	HashTable *lastSymTable = NULL;
+	zend_execute_data *ex = coroutine->execute_data;
+	for (; ex; ex = ex->prev_execute_data) {
+		HashTable *symTable;
+		if (ZEND_CALL_INFO(ex) & ZEND_CALL_GENERATOR) {
+			zend_generator *generator = (zend_generator*)ex->return_value;
+			if (!(generator->flags & ZEND_GENERATOR_CURRENTLY_RUNNING)) {
+				continue;
+			}
+			symTable = zend_generator_frame_gc(buf, generator);
+		} else {
+			symTable = zend_unfinished_execution_gc_ex(ex, 
+				ex->func && ZEND_USER_CODE(ex->func->type) ? ex->call : NULL, 
+				buf, false);
+		}
+		if (symTable) {
+			if (lastSymTable) {
+				zval *val;
+				ZEND_HASH_FOREACH_VAL(lastSymTable, val) {
+					if (EXPECTED(Z_TYPE_P(val) == IS_INDIRECT)) {
+						val = Z_INDIRECT_P(val);
+					}
+					zend_get_gc_buffer_add_zval(buf, val);
+				} ZEND_HASH_FOREACH_END();
+			}
+			lastSymTable = symTable;
+		}
+	}
+
+	zend_get_gc_buffer_use(buf, table, num);
+	return lastSymTable;
+}
+
 static zend_object_handlers coroutine_handlers;
 
 void async_register_coroutine_ce(void)
@@ -1197,6 +1330,7 @@ void async_register_coroutine_ce(void)
 	coroutine_handlers.clone_obj = NULL;
 	coroutine_handlers.dtor_obj = coroutine_object_destroy;
 	coroutine_handlers.free_obj = coroutine_free;
+	coroutine_handlers.get_gc = async_coroutine_object_gc;
 }
 
 //////////////////////////////////////////////////////////////////////

scope.c

@@ -1229,6 +1229,84 @@ static void scope_destroy(zend_object *object)
 	}
 }
 
+static HashTable *scope_object_gc(zend_object *object, zval **table, int *num)
+{
+	async_scope_object_t *scope_obj = (async_scope_object_t *)object;
+	async_scope_t *scope = scope_obj->scope;
+
+	if (scope == NULL) {
+		*table = NULL;
+		*num = 0;
+		return NULL; // No scope to collect
+	}
+
+	zend_get_gc_buffer *buf = zend_get_gc_buffer_create();
+
+	/* Add exception handler ZVALs if present */
+	if (scope->exception_fci) {
+		zend_get_gc_buffer_add_zval(buf, &scope->exception_fci->function_name);
+	}
+
+	/* Add child exception handler ZVALs if present */
+	if (scope->child_exception_fci) {
+		zend_get_gc_buffer_add_zval(buf, &scope->child_exception_fci->function_name);
+	}
+
+	/* Add finally handlers if present */
+	if (scope->finally_handlers) {
+		zval *val;
+		ZEND_HASH_FOREACH_VAL(scope->finally_handlers, val) {
+			zend_get_gc_buffer_add_zval(buf, val);
+		} ZEND_HASH_FOREACH_END();
+	}
+
+	/* Add context ZVALs if present */
+	if (scope->scope.context) {
+		/* Cast to actual context implementation to access HashTables */
+		async_context_t *context = (async_context_t *)scope->scope.context;
+		
+		/* Add all values from context->values HashTable */
+		zval *val;
+		ZEND_HASH_FOREACH_VAL(&context->values, val) {
+			zend_get_gc_buffer_add_zval(buf, val);
+		} ZEND_HASH_FOREACH_END();
+		
+		/* Add all object keys from context->keys HashTable */
+		ZEND_HASH_FOREACH_VAL(&context->keys, val) {
+			zend_get_gc_buffer_add_zval(buf, val);
+		} ZEND_HASH_FOREACH_END();
+	}
+
+	zend_get_gc_buffer_use(buf, table, num);
+	return NULL;
+}
+
+static void scope_object_free(zend_object *object)
+{
+	async_scope_object_t *scope_object = (async_scope_object_t *) object;
+
+	async_scope_t *scope = scope_object->scope;
+
+	if (scope == NULL) {
+		return;
+	}
+
+	scope_object->scope = NULL;
+	scope->scope.scope_object = NULL;
+	zend_object_std_dtor(&scope_object->std);
+
+	// 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.
+	if (false == scope->scope.try_to_dispose(&scope->scope)) {
+		zend_object *exception = async_new_exception(
+			async_ce_cancellation_exception, "Scope is being disposed due to object destruction"
+		);
+
+		ZEND_ASYNC_SCOPE_CANCEL(&scope->scope, exception, true, ZEND_ASYNC_SCOPE_IS_DISPOSE_SAFELY(&scope->scope));
+	}
+}
+
 void async_register_scope_ce(void)
 {
 	async_ce_scope_provider = register_class_Async_ScopeProvider();
@@ -1242,6 +1320,8 @@ void async_register_scope_ce(void)
 
 	async_scope_handlers.clone_obj = NULL;
 	async_scope_handlers.dtor_obj = scope_destroy;
+	async_scope_handlers.get_gc = scope_object_gc;
+	async_scope_handlers.free_obj = scope_object_free;
 }
 
 /**

tests/coroutine/019-coroutine_gc_basic.phpt

@@ -0,0 +1,29 @@
+--TEST--
+Coroutine: GC handler basic functionality
+--FILE--
+<?php
+
+use function Async\spawn;
+use function Async\suspend;
+
+// Test that GC handler is registered and functioning
+$coroutine = spawn(function() {
+    return "test_value";
+});
+
+// Force garbage collection to ensure our GC handler is called
+$collected = gc_collect_cycles();
+
+suspend(); // Suspend to simulate coroutine lifecycle
+
+// Check that coroutine completed successfully
+$result = $coroutine->getResult();
+var_dump($result);
+
+// Verify GC was called (should return >= 0)
+var_dump($collected >= 0);
+
+?>
+--EXPECT--
+string(10) "test_value"
+bool(true)

tests/coroutine/020-coroutine_gc_with_finally.phpt

@@ -0,0 +1,35 @@
+--TEST--
+Coroutine: GC handler with finally handlers
+--FILE--
+<?php
+
+use function Async\spawn;
+use function Async\suspend;
+
+// Test GC with finally handlers containing callable ZVALs
+$coroutine = spawn(function() {
+    return "test_value";
+});
+
+// Add finally handler with callable
+$coroutine->onFinally(function() {
+    echo "Finally executed\n";
+});
+
+// Force garbage collection
+$collected = gc_collect_cycles();
+
+suspend(); // Suspend to simulate coroutine lifecycle
+
+// Wait for completion
+$result = $coroutine->getResult();
+var_dump($result);
+
+// Verify GC was called
+var_dump($collected >= 0);
+
+?>
+--EXPECT--
+Finally executed
+string(10) "test_value"
+bool(true)

tests/coroutine/021-coroutine_gc_with_context.phpt

@@ -0,0 +1,45 @@
+--TEST--
+Coroutine: GC handler with context data
+--FILE--
+<?php
+
+use function Async\spawn;
+use function Async\suspend;
+use Async\Context;
+
+// Test GC with coroutine context containing ZVALs
+$context = new Context();
+$context->set("string_key", "string_value");
+
+// Test object key as well
+$obj_key = new stdClass();
+$context->set($obj_key, "object_value");
+
+$coroutine = spawn(function() use ($context, $obj_key) {
+    // Access context to ensure it's tracked by GC
+    $string_val = $context->get("string_key");
+    $obj_val = $context->get($obj_key);
+    return [$string_val, $obj_val];
+});
+
+// Force garbage collection to test context ZVAL tracking
+$collected = gc_collect_cycles();
+
+suspend(); // Suspend to simulate coroutine lifecycle
+
+// Get result
+$result = $coroutine->getResult();
+var_dump($result);
+
+// Verify GC was called
+var_dump($collected >= 0);
+
+?>
+--EXPECT--
+array(2) {
+  [0]=>
+  string(12) "string_value"
+  [1]=>
+  string(12) "object_value"
+}
+bool(true)