Skip to content

Correct context, AOP, and event execution semantics#437

Merged
binaryfire merged 21 commits into
0.4from
audit/context-di-events-runtime-safety
Jul 16, 2026
Merged

Correct context, AOP, and event execution semantics#437
binaryfire merged 21 commits into
0.4from
audit/context-di-events-runtime-safety

Conversation

@binaryfire

Copy link
Copy Markdown
Collaborator

Summary

This PR corrects three related runtime boundaries that become observable in a long-lived Swoole worker: explicit coroutine context ownership, generated AOP proxy semantics, and mutable event or queue execution state.

The previous implementations were individually workable on ordinary paths, but they relied on assumptions that do not hold under explicit cross-coroutine access, cached service reuse, generated method rewriting, or concurrent queue execution. This change establishes the correct ownership model at each lowest boundary and removes the compensating or duplicate machinery that is no longer needed.

The result is:

  • explicit coroutine IDs address exactly the requested live coroutine and never fall through to worker-global fallback storage;
  • generated AOP proxies preserve native PHP method behavior while using content-addressed, atomically published artifacts;
  • queued listeners and mapped handlers isolate per-job mutable state without giving up worker-cached dependencies;
  • event dispatch, after-commit observers, backoff metadata, queue options, job chains, and lock keys follow current Laravel behavior;
  • filesystem directory creation now guarantees its documented postcondition;
  • coroutine-heavy tests propagate child failures and release everything they own.

For more details, see: docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md

Context ownership

CoroutineContext previously selected storage from the caller's execution mode even when the caller supplied another coroutine ID. An outside caller could therefore fail to address a live coroutine, while destructive operations aimed at a dead coroutine could affect worker-global fallback state instead.

Context storage now follows one explicit rule:

  • an omitted coroutine ID uses the current coroutine or the non-coroutine fallback;
  • an explicit ID addresses only that coroutine;
  • explicit reads from a destroyed coroutine return their empty result;
  • explicit writes to a destroyed coroutine raise the existing CoroutineDestroyedException;
  • flushes clear a live target in place instead of replacing unrelated storage.

Bulk context writes also normalize integer array keys before crossing the string-or-enum key API. Documentation and database transaction test bridges now state clearly that non-coroutine copy operations are testing lifecycle tools, not request-scoped storage.

Native AOP semantics

The old proxy generator rewrote intercepted method bodies into per-invocation closures. That changes native PHP behavior in subtle but important ways: method-local statics, references, variadics, argument introspection, magic constants, closure descriptors, default objects, and void or never control flow can all observe the generated helper rather than the original method.

The new generator preserves the public method signature and moves the original body into a collision-checked private helper. A stateless dispatcher owns advice execution, while the generated wrapper remains responsible only for forwarding the native call shape.

Proxy identity and publication are corrected at the same boundary:

  • fingerprints include every input that can change generated output;
  • encoded, content-addressed filenames prevent stale or colliding artifacts;
  • source reads are checked;
  • files are published atomically;
  • Composer mappings are updated only after the complete generation batch succeeds;
  • an empty marker trait identifies generated classes without the duplicate runtime behavior previously carried by ProxyTrait;
  • aspect discovery reuses the canonical immutable reflection metadata cache.

The old trait dispatch path and its duplicate Foundation testing implementation are removed completely.

Events and queue execution

Worker-cached listeners and mapped queue handlers can be resolved once and then reused concurrently. Injecting the current job directly into those shared objects allowed one execution to overwrite another execution's job handle.

The dispatcher and queue handler now clone only resolved objects that use InteractsWithQueue before assigning per-job state. Stateless listeners and handlers continue to reuse the cached instance, and configured dependencies remain shared normally.

The surrounding event and queue corrections complete the same execution contract:

  • event helpers forward positional and named constructor arguments;
  • cancellable pre-mutation model observer hooks remain synchronous even when the observer requests after-commit handling;
  • post-mutation hooks remain deferred until commit;
  • queued event cloning preserves enum cases;
  • queued closures retain every valid callable deduplicator and supported message-group value;
  • queued listener payloads use the array form emitted by every current producer, removing the obsolete serialized-string branch;
  • the variadic Backoff attribute supports scalar, array, positional, and named forms without the upstream named-variadic indexing failure;
  • PendingChain and its fake forward variadic constructor arguments consistently;
  • unique-job and overlap lock keys use Laravel's current xxh128 display-name hashing for cross-framework queue compatibility.

Filesystem boundary

ensureDirectoryExists() could silently return after a failed recursive mkdir, leaving callers to fail later and farther from the actual cause. It now accepts the valid concurrent-creator race by rechecking the directory after the native call, then throws the existing filesystem exception if the postcondition still is not satisfied.

Performance and compatibility

The changes preserve Laravel-facing public APIs and configuration. Most surface changes restore current Laravel behavior or broaden existing forwarding types to values queue execution already supports.

The AOP path removes per-invocation closure construction and centralizes dispatch in a stateless runtime component. Content hashing and source validation happen during proxy generation, not request execution. Context targeting adds no registry or synchronization layer. Listener and handler cloning occurs only for the mutable InteractsWithQueue case; stateless cached consumers retain the existing fast path.

No request-wide locks, retries, polling, context registries, compatibility adapters, or generalized lifecycle framework are introduced.

Validation

The implementation includes focused regressions for explicit live and destroyed context targets, fallback isolation, native-versus-proxied PHP behavior, proxy fingerprint invalidation and publication, listener and handler interleaving, transaction timing, queue option forwarding, backoff forms, named job construction, lock-key interoperability, filesystem creation races, and failure-safe coroutine tests.

The complete formatting, static-analysis, parallel component, and Testbench gates pass for the committed state.

Resolve context storage from the requested coroutine rather than from the caller's execution mode.

Treat an omitted coroutine ID as the current-context-or-fallback operation, while explicit IDs address only their target. Dead explicit writes now raise the existing CoroutineDestroyedException, reads return their empty result, and destructive operations cannot fall through to worker-global storage.

Flush live targets in place and normalize integer bulk keys before delegating to the string and enum key API.
Add deterministic regressions for live suspended targets, current-coroutine mutation, destroyed explicit targets, in-place flushing, and integer-backed enum bulk keys.

Capture non-coroutine bridge results and assert them after child completion so coroutine exception handling cannot hide PHPUnit failures.

Explicitly drain outside-coroutine Swoole work owned by the tests to avoid shutdown-time event-loop cleanup.
Document the complete behavior of explicit live and destroyed coroutine targets.

Frame the non-coroutine copy and clear operations around their real test-lifecycle ownership, and add matching tests-only warnings to the database transaction bridge so request code does not mistake worker-global storage for request-scoped state.

Record the Context package's Hyperf upstream reference.
Record the verified Context defects, final implementation boundary, rejected complexity, regression coverage, validation, performance assessment, and owner approval.

Mark Context complete, close the Container revalidation work, add the new cross-package dependencies for Foundation and Database, and route the next audit slice to DI with its Reflection dependency.
Make ensureDirectoryExists establish its documented postcondition instead of silently continuing after a failed recursive mkdir.

Accept the valid concurrent-creator race by checking the directory again after the native call, and throw the existing filesystem exception when creation genuinely failed. Add regressions for successful creation, concurrent creation, and terminal failure.
Route service-provider aspect discovery through ClassMetadataCache so DI uses the framework-wide immutable reflection owner instead of constructing duplicate ReflectionClass instances.

Keep class-map mutation tests isolated from Composer’s worker loader and restore the exact original loader after each test, preventing test-only entries from leaking into later application bootstraps.
Replace the per-invocation closure rewrite with a signature-preserving wrapper and collision-checked private helper. Preserve method-local statics, default objects, references, variadics, argument introspection, magic constants, closure descriptors, and void or never control flow.

Consolidate runtime advice execution in a stateless dispatcher and identify generated classes, traits, enums, aliases, and inherited proxies with an empty marker trait. Remove the collision-prone ProxyTrait path and its duplicate Foundation testing implementation.

Make proxy artifacts content-addressed across every generation input, use encoded filenames and checked source reads, publish files atomically, and update the Composer map only after the complete batch succeeds. Add native-versus-proxied semantic coverage, fingerprint invalidation regressions, loader isolation, marker composition, and deterministic rejection of source forms that cannot be preserved safely.
Explain the complete fingerprint used to reuse generated proxies and the source shapes that can be intercepted without changing native PHP behavior.

Update the earlier runtime-isolation design record so its description of source-map recovery, explicit source inputs, loader ownership, and cache invalidation matches the final content-addressed implementation.
Capture the verified proxy semantic, artifact publication, filesystem, and test-isolation defects together with their final implementation boundaries and rejected complexity.

Record native-equivalence coverage, the complete green validation gate, independent review, unchanged public AOP surface, and the measured intercepted-path improvement. Mark DI complete and route the next audit cycle to Events with its pending Reflection revalidation.
Complete the audit ledger after the owner reviewed the signed-off DI and AOP work unit and authorized committing its source, tests, documentation, and bookkeeping records.
Allow Dispatchable event helpers to forward positional and named constructor arguments directly. This brings dispatch and broadcast behavior in line with current Laravel and preserves PHP's named-argument semantics.

Add focused dispatch and broadcast coverage, and pin the corrected closure-listener first-parameter inference at the Events consumer boundary.
Keep cancellable pre-mutation model observer hooks synchronous even when an observer requests after-commit handling. This preserves the return value that controls the model operation while leaving post-mutation hooks deferred until commit.

Also preserve immutable enum cases at the queued-event clone boundary and correct listener registry annotations for callable and Class@method forms. Add transaction regressions for immediate creating and updating hooks, deferred post-events, and operation cancellation.
Clone only container-resolved listeners that use InteractsWithQueue before injecting the current job. This prevents concurrent jobs from overwriting a worker-shared listener's mutable job handle while retaining shared configured dependencies and normal stateless listener reuse.

Make listener payloads array-only to match every current producer, remove the obsolete serialized-string compatibility path, preserve enum cases during payload cloning, and widen backoff storage for supported retry sequences. Add deterministic coroutine coverage for per-job ownership and object payload cloning.
Preserve every valid callable deduplicator shape instead of narrowing queued closures to Closure-only storage. Align PendingDispatch with Queueable so callable arrays and all supported message-group values pass through unchanged.

Retain integer values produced by int-backed enum groups and add unit and end-to-end coverage for array-callable deduplicators, array groups, and enum-backed message groups.
Clone container-resolved mapped handlers that use InteractsWithQueue before injecting the current job. This closes the same worker-shared job-handle race as queued listeners without changing fresh deserialized command handling or stateless mapped handlers.

Add a deterministic concurrent regression that proves each mapped handler execution retains its own job instance.
Port the current variadic Backoff attribute and preserve the existing scalar and array forms. Normalize variadic keys so named scalar and named array arguments remain valid instead of triggering the named-variadic indexing bug present upstream.

Widen queued listener and broadcast job backoff storage to the retry sequences already accepted by queue execution. Add coverage for positional, variadic, named, method-provided, listener, broadcast, and enum-serialization paths.
Hash custom job display names with xxh128 when building unique-job and overlap lock keys. This matches current Laravel exactly so Hypervel and Laravel workers acquire and release the same lock for shared queues.

Update unique job, overlapping middleware, broadcasting, and interoperability regressions to assert the exact cross-framework keys rather than only their prefixes.
Make the real PendingChain and its testing fake accept the same variadic constructor arguments when the first chained job is a class name. This fixes named first-job construction while keeping both overrides signature-compatible.

Add real worker and Bus fake regressions to prove named arguments reach the first job in both execution paths.
Replace manual WaitGroup bookkeeping with the framework parallel helper in event isolation tests. Child exceptions now propagate to the test runner and all children are joined, so a failed assertion or dispatch cannot strand the suite waiting on a missing done call.

The tested event isolation and interleaving behavior remains unchanged while the test lifecycle becomes smaller and deterministic.
Record the Events package's Laravel provenance and explain why Hypervel omits the obsolete serialized-string queued-listener payload path. Every supported producer already emits the current array payload.

Document scalar, array, and variadic Backoff attribute forms for jobs and queued listeners, and correct the surrounding listener wording.
Capture the verified Events, Queue, Broadcasting, Foundation, Bus, and Support findings, accepted boundaries, rejected complexity, implementation, regression coverage, performance assessment, and completed validation.

Mark Events complete, close its Reflection revalidation, add the shared ownership routes needed by later package audits, and route the next work unit to Log with its Container dependencies.
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 62 files, which is 12 over the limit of 50.

To get a review, narrow the scope:
• coderabbit review --type committed # exclude uncommitted changes
• coderabbit review --dir # limit to a subdirectory
• coderabbit review --base # compare against a closer base

Upgrade to a paid plan to raise the limit.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a6577af4-17c6-4e2c-888c-ce387b52f03e

📥 Commits

Reviewing files that changed from the base of the PR and between 16a1226 and b307430.

📒 Files selected for processing (66)
  • docs/plans/2026-07-05-parallel-redis-runtime-isolation.md
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md
  • docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit.md
  • src/boost/docs/aop.md
  • src/boost/docs/coroutine-context.md
  • src/boost/docs/events.md
  • src/boost/docs/queues.md
  • src/broadcasting/src/BroadcastEvent.php
  • src/bus/src/UniqueLock.php
  • src/context/README.md
  • src/context/src/CoroutineContext.php
  • src/database/src/DatabaseTransactionsManager.php
  • src/di/src/Aop/AspectCollector.php
  • src/di/src/Aop/AspectManager.php
  • src/di/src/Aop/Ast.php
  • src/di/src/Aop/ProceedingJoinPoint.php
  • src/di/src/Aop/ProxyCallVisitor.php
  • src/di/src/Aop/ProxyDispatcher.php
  • src/di/src/Aop/ProxyManager.php
  • src/di/src/Aop/ProxyMarker.php
  • src/di/src/Aop/ProxyTrait.php
  • src/di/src/Aop/VisitorMetadata.php
  • src/events/README.md
  • src/events/src/CallQueuedListener.php
  • src/events/src/Dispatcher.php
  • src/events/src/QueuedClosure.php
  • src/filesystem/src/Filesystem.php
  • src/foundation/src/Bus/PendingChain.php
  • src/foundation/src/Bus/PendingDispatch.php
  • src/foundation/src/Events/Dispatchable.php
  • src/foundation/src/Testing/Concerns/InteractsWithAop.php
  • src/queue/src/Attributes/Backoff.php
  • src/queue/src/CallQueuedHandler.php
  • src/queue/src/Middleware/WithoutOverlapping.php
  • src/support/src/ServiceProvider.php
  • src/support/src/Testing/Fakes/PendingChainFake.php
  • tests/Broadcasting/BroadcastEventTest.php
  • tests/Bus/BusPendingDispatchTest.php
  • tests/Context/ContextEnumTest.php
  • tests/Context/ContextTest.php
  • tests/Di/Aop/AspectManagerTest.php
  • tests/Di/Aop/ProceedingJoinPointTest.php
  • tests/Di/Aop/ProxyCallVisitorTest.php
  • tests/Di/Aop/ProxyDispatcherTest.php
  • tests/Di/Aop/ProxyTraitTest.php
  • tests/Di/Bootstrap/GenerateProxiesTest.php
  • tests/Di/ClassMap/ClassMapManagerTest.php
  • tests/Di/Fixtures/ProxyTraitObject.php
  • tests/Di/Fixtures/ProxyTraitOnTrait.php
  • tests/Di/ServiceProviderDiTest.php
  • tests/Events/BroadcastedEventsTest.php
  • tests/Events/CallQueuedListenerTest.php
  • tests/Events/CoroutineEventsTest.php
  • tests/Events/EventsDispatcherTest.php
  • tests/Events/QueuedEventsTest.php
  • tests/Filesystem/FilesystemTest.php
  • tests/Foundation/Testing/Concerns/InteractsWithAopTest.php
  • tests/Integration/Broadcasting/BroadcastManagerTest.php
  • tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php
  • tests/Integration/Events/QueuedClosureListenerTest.php
  • tests/Integration/Queue/JobChainingTest.php
  • tests/Integration/Queue/UniqueJobTest.php
  • tests/Integration/Queue/WithoutOverlappingJobsTest.php
  • tests/Queue/CallQueuedHandlerTest.php
  • tests/Queue/LaravelInteropTest.php
  • tests/Queue/ReadsQueueAttributesTest.php

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Review on demand using usage pricing
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch audit/context-di-events-runtime-safety

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown

Greptile Summary

This PR corrects three coroutine-safety boundaries that were observable in long-lived Swoole workers — explicit coroutine context ownership, generated AOP proxy semantics, and mutable event/queue execution state — along with a cluster of surrounding correctness fixes.

  • CoroutineContext: All storage operations now route on Coroutine::getContextFor() rather than Coroutine::id() > 0, so explicit coroutine IDs address only that coroutine's storage; reads from a destroyed coroutine return empty, writes throw; flush() uses exchangeArray([]) atomically; and a pre-existing bug in forget() (always clearing the non-coroutine store) is fixed.
  • AOP proxy generation: ProxyCallVisitor now keeps the original method body in a collision-checked private helper and emits a thin public wrapper that delegates to the new stateless ProxyDispatcher. This preserves func_get_args/arg/num, magic constants, statics, and void/never semantics. ProxyManager switches to content-addressed, atomically-written proxy files whose fingerprints cover source content, visitors, framework AOP source, php-parser version, and PHP version.
  • Event/queue isolation: Resolved listeners and mapped handlers that use InteractsWithQueue are cloned before setJob() to prevent one concurrent execution from overwriting another's job handle. Pre-mutation Eloquent observer hooks are excluded from after-commit deferral so they retain their cancellation ability. Enum instances are excluded from clone-on-dispatch. Dispatchable::dispatch(), PendingChain::dispatch(), and broadcast() now forward arguments through variadic parameters instead of broken func_get_args() calls inside no-arg methods. Backoff supports scalar, array, and multi-value positional forms. ensureDirectoryExists() throws on creation failure instead of silently returning.

Confidence Score: 4/5

Safe to merge; the two lock-key findings are minor consistency issues in an uncommon fallback path.

The large majority of changes are well-reasoned semantic corrections addressing real observable bugs in Swoole workers. The only outstanding finding is a key-format inconsistency in UniqueLock and WithoutOverlapping where jobs that do not implement displayName() produce an un-hashed lock key while all other paths produce a hashed one. This affects only the uncommon fallback branch and does not cause data loss, but it could cause a cross-framework queue consumer to fail to recognise the lock entry.

src/bus/src/UniqueLock.php and src/queue/src/Middleware/WithoutOverlapping.php have the fallback get_class() path that skips the xxh128 hash added everywhere else.

Important Files Changed

Filename Overview
src/context/src/CoroutineContext.php Rewrites all storage operations to use Coroutine::getContextFor() return value as the routing branch, eliminating the old Coroutine::id() > 0 check that silently fell through to worker-global storage when an explicit cross-coroutine ID addressed a destroyed coroutine. flush() now uses exchangeArray([]) for an atomic clear. forget() bug (always clearing non-coroutine store) is fixed.
src/di/src/Aop/ProxyCallVisitor.php Major rewrite: keeps the original method body in a collision-checked private helper and generates a thin public wrapper delegating to ProxyDispatcher::dispatch(). Fixes func_get_args/num/arg, __FILE__, __DIR__, __FUNCTION__, __METHOD__, and __LINE__ semantics. Indirect call_user_func('func_get_args', …) is explicitly rejected.
src/di/src/Aop/ProxyDispatcher.php New stateless dispatcher replacing ProxyTrait. Aspect list is resolved and cached atomically. resolveArguments/resolveArgument correctly reconstruct func_get_args/arg semantics from the original call count.
src/di/src/Aop/ProxyManager.php Content-addressed proxy files: fingerprint covers source content, visitors, framework AOP sources, aspect rules, php-parser version, and PHP version ID. Proxy directory ensured before generation. File naming uses rawurlencode() instead of backslash-to-underscore.
src/events/src/CallQueuedListener.php Removes serialized-string payload branch. Fixes InteractsWithQueue shared-instance race by cloning before setJob(). Fixes enum cloning in __clone().
src/events/src/Dispatcher.php Pre-mutation observer hooks excluded from after-commit deferral so they retain cancellation ability. Enum instances excluded from clone-on-dispatch. Type annotations tightened.
src/queue/src/CallQueuedHandler.php Fixes worker-shared handler race: mapped handlers using InteractsWithQueue are cloned before setJob().
src/filesystem/src/Filesystem.php ensureDirectoryExists() re-checks isDirectory() after failed mkdir() to tolerate concurrent-creator races, then throws RuntimeException if postcondition still not met.
src/bus/src/UniqueLock.php Adds xxh128 hashing of displayName() to match Laravel's lock key format. Fallback get_class() path is not hashed, leaving a minor key-shape inconsistency.
src/queue/src/Attributes/Backoff.php Constructor changed to variadic `array
src/foundation/src/Events/Dispatchable.php dispatch() and broadcast() now forward constructor arguments via mixed ...$arguments, replacing broken func_get_args() inside a no-parameter method.
src/foundation/src/Bus/PendingChain.php dispatch() now accepts mixed ...$arguments instead of relying on func_get_args() inside a no-arg method, fixing argument forwarding when the job is a string class name.

Comments Outside Diff (1)

  1. src/bus/src/UniqueLock.php, line 60-65 (link)

    P2 Lock key asymmetry in fallback path

    When a job does not implement displayName(), the fallback get_class($job) value is inserted un-hashed into the lock key, while jobs that do implement displayName() get the hashed form. The same asymmetry exists in WithoutOverlapping::getLockKey(). A job class whose name happened to produce the same 32-char hex string as another job's hashed display name would collide; more practically, any cross-framework tooling that expects laravel_unique_job:{xxh128-hash}:… would receive a raw class name in this branch and could fail to recognise the key as a unique-job entry.

    Fix in Claude Code Fix in Codex

Fix All in Claude Code Fix All in Codex

Reviews (1): Last reviewed commit: "Record the completed Events audit" | Re-trigger Greptile

Comment thread src/queue/src/Middleware/WithoutOverlapping.php
@binaryfire

Copy link
Copy Markdown
Collaborator Author

@greptileai I checked the inline finding and the matching outside-diff item against the committed PR snapshot, Laravel 13.x, its regression tests, and the upstream change that introduced this behavior.

Both comments describe the same intentional key contract:

  • Custom displayName() values are hashed because they can contain Memcached-invalid characters.
  • The get_class() fallback remains raw because PHP class names are already safe and Laravel uses that exact form.
  • UniqueLock, WithoutOverlapping, and DebounceLock all follow this pattern.
  • Existing tests explicitly cover the raw class-name fallback and the hashed custom-name path.

Applying the suggested hash to the fallback would change keys for ordinary jobs without displayName() and break Laravel/Hypervel lock interoperability during mixed consumption or deployment. No source or test change is appropriate here. There were no other actionable findings in the review.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown

Acknowledged — that's the correct read. The asymmetry is load-bearing: displayName() can return arbitrary strings that contain characters Memcached rejects, so hashing is required there; PHP class names are already safe, and hashing the fallback would silently rotate keys for every ordinary job, breaking lock interoperability between Laravel and Hypervel consumers during any mixed deployment or rolling restart.

My suggestion was wrong. No change needed here.

@binaryfire
binaryfire merged commit 1f1f833 into 0.4 Jul 16, 2026
35 of 36 checks passed
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.

1 participant