Correct context, AOP, and event execution semantics#437
Conversation
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.
|
Important Review skippedToo many files! This PR contains 62 files, which is 12 over the limit of 50. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (66)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis 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.
Confidence Score: 4/5Safe 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 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
|
|
@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:
Applying the suggested hash to the fallback would change keys for ordinary jobs without |
|
Acknowledged — that's the correct read. The asymmetry is load-bearing: My suggestion was wrong. No change needed here. |
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:
For more details, see:
docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.mdContext ownership
CoroutineContextpreviously 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:
CoroutineDestroyedException;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
voidornevercontrol 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:
ProxyTrait;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
InteractsWithQueuebefore 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:
xxh128display-name hashing for cross-framework queue compatibility.Filesystem boundary
ensureDirectoryExists()could silently return after a failed recursivemkdir, 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
InteractsWithQueuecase; 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.