Skip to content

Conversation

@EdmondDantes
Copy link
Contributor

No description provided.

…bjects

  - Add async_coroutine_object_gc() handler for complete coroutine GC tracking
    * Track all ZVALs: result, exception, deferred_cancellation
    * Track finally_handlers, internal_context, and fcall parameters
    * Track waker events, error objects, and triggered events
    * Track context values/keys HashTables and scope objects
    * Support execution stack traversal for suspended coroutines

  - Add async_scope_object_gc() handler for scope GC tracking
    * Track exception and child exception handler ZVALs
    * Track finally_handlers HashTable with callable ZVALs
    * Track context values/keys HashTables

  - Register GC handlers in coroutine_handlers and async_scope_handlers
  - Add comprehensive test coverage (15 new tests) for GC functionality
  - Update project documentation and CHANGELOG for v0.3.0
  - Prevent memory leaks in complex async applications with circular references

  This implementation follows PHP's fiber GC pattern and ensures proper
  memory management for all async object hierarchies and nested structures.
@EdmondDantes EdmondDantes requested a review from Copilot July 2, 2025 10:53
@EdmondDantes EdmondDantes self-assigned this Jul 2, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances garbage collection and finalizer support in the async extension by tracking internal ZVAL references in both Scope and Coroutine objects, and adds comprehensive tests to cover these new behaviors.

  • Added get_gc handlers (scope_object_gc and async_coroutine_object_gc) to track ZVALs for scopes and coroutines, including finally handlers, exceptions, contexts, and waker structures.
  • Introduced new PHPT tests for finally handlers, GC scenarios, and error conditions (e.g., wrong constructor).
  • Updated READMEs and CHANGELOG to reflect new API support and test coverage.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/scope/README.md Updated list with Finally Handlers and Garbage Collection tests
tests/scope/021-scope_with_wrong_constructor.phpt Added new test for faulty constructor memory cleanup
tests/scope/019-scope_gc_basic.phpt New test for basic GC handler functionality
tests/scope/020-scope_gc_with_finally.phpt New test combining GC and finally handlers
tests/scope/021-scope_gc_with_context.phpt New test for GC handling of context data
tests/coroutine/README.md Expanded list with Finally Handlers and GC tests for coroutines
tests/coroutine/019-025 *.phpt Multiple new coroutine GC and finally-handler tests
scope.c Implemented scope_object_gc and updated object handlers
coroutine.c Implemented async_coroutine_object_gc and updated handlers
CHANGELOG.md Documented GC support and memory management improvements
Comments suppressed due to low confidence (4)

tests/scope/README.md:21

  • [nitpick] The test filename mixes snake_case (on_finally) with camelCase (onFinally). Consider renaming to 013-scope_on_finally_execution.phpt for consistency with existing test naming.
- **013-scope_onFinally_execution.phpt**: Finally handlers execution testing

tests/scope/021-scope_with_wrong_constructor.phpt:2

  • The test description does not match its purpose (testing a faulty constructor and memory cleanup). Update the header to reflect "wrong constructor" scenario.
Scope: GC handler with context data

tests/scope/README.md:28

  • The new 021-scope_with_wrong_constructor.phpt test is not listed under any section. Add it under the appropriate section in this README to ensure it's discoverable.
### Garbage Collection

coroutine.c:1278

  • This condition references coroutine->context.status, but the status field lives in coroutine->coroutine.status. Using the wrong struct member will prevent proper traversal of suspended execution frames.
	if (coroutine->context.status != ZEND_FIBER_STATUS_SUSPENDED || 

@EdmondDantes EdmondDantes merged commit 07fcd93 into main Jul 2, 2025
1 of 2 checks passed
@EdmondDantes EdmondDantes deleted the 6-proper-handling-of-internal-references-to-zvals-for-coroutines-and-scopes branch July 2, 2025 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proper handling of internal references to ZVALs for coroutines and Scopes

2 participants