Harden coroutine runtime and resource lifecycle - #440
Conversation
Model stream detachment explicitly so size, readability, and writability remain truthful after the underlying contents are released. Reject negative reads without mutating the buffer, preserve zero-length reads, maintain the remaining size directly, and return PSR-compatible metadata results instead of throwing an unrelated implementation exception. Add focused regressions for size tracking, invalid reads, metadata, and every operation that must fail after detachment.
Check the native Swoole HTTP/2 connect result during client construction and convert a false result into the existing HttpClientException contract with the native error details. This prevents an unusable client from escaping construction and failing later at a less useful boundary. Add a deterministic regression covering immediate connection refusal.
Return native Swoole push results instead of reporting unconditional success, reject failed upgrades at construction, and clear connection-local callbacks and handles through one exception-safe lifecycle boundary. Simplify Frame and its contract to represent only capabilities Swoole actually supports: compute payload length from the payload, expose the native boolean mask flag, remove the ineffective masking-key and payload-length mutators, and drop the ignored serialization argument. Expand regressions to cover both native push branches, upgrade failure, cleanup after callback failure, computed payload length, immutable mask changes, and the mask bit emitted on the wire.
Delete the unused HTTP server, factory, response emitter, event stream, and their contracts that were inherited from Hyperf's interchangeable Swoole and Swow architecture. Hypervel permanently targets Swoole, and its live HTTP, WebSocket, and Reverb servers already use the canonical Swoole request and response bridges in hypervel/http-server. Remove the dead provider binding, direct package dependencies, obsolete regression, and lifecycle-plan claims that applied only to the deleted server. Document Engine's upstream origin and the intentional Swoole-only architecture so future ports do not restore the portability layer.
Document the verified Engine defects, the owner-approved Swoole-only simplification, rejected speculative complexity, implementation boundaries, regression coverage, validation, API impact, and performance assessment. Mark Engine complete in the framework-wide audit and advance the active package to Coroutine. No cross-package revalidation remains from this work unit.
Introduce a keys-first captureFrom API that snapshots and replicates context in the calling coroutine before any destination is modified. Keep copyFrom compatible while routing installation through setMany so explicit destinations remain isolated from non-coroutine fallback storage. Add direct coverage for current, filtered, explicit, dead, and non-coroutine source behavior, and prove that replication failures cannot partially overwrite the destination. Refresh the log context comments to describe the generalized replication boundary.
Capture parent context synchronously before spawning so replication failures remain caller-visible and concurrency capacity or wait counts can roll back cleanly. Remove duplicate Concurrent exception reporting and make the high-level coroutine reporter resilient when container or handler infrastructure also fails. Correct run() callable-array handling and restore the exact prior Swoole hook flags in a finally block. Add deterministic regressions for reporting failure, context replication failure, balanced Concurrent and Parallel bookkeeping, callable tuples, and hook restoration.
Construct the replacement instance and its channel before publishing either one. A failed factory now leaves the previously published caller intact instead of closing healthy state before a replacement exists. Cover both successful replacement cleanup and factory failure so the channel transaction cannot regress.
Remove Locker and Mutex entries when their channels are released instead of retaining null tombstones for every dynamic key. Narrow the maps to their truthful channel-only type and keep flushState responsible for closing only live channels. Add regressions that inspect released-key removal and prevent unbounded worker-lifetime map growth from returning.
Give WaitConcurrent::fork() the same wait-group ownership contract as create(): reserve before spawning, finish in the child finally block, and roll the reservation back when synchronous context capture or coroutine creation fails. Cover ordinary fork completion and synchronous failure so wait() cannot return while forked work remains active or hang on a stranded count.
Record whether each atomic filesystem boundary actually acquired its coroutine lock and release it only in that case. Cancellation while waiting can no longer unlock another coroutine's live critical section. Exercise both Filesystem and LockableFile with deterministic owner/waiter scheduling to prove canceled waiters never release a gate they do not own.
Snapshot and replicate the current context before allocating the result channel or creating the child coroutine. Replication failures now surface immediately instead of being reported in a child and misdiagnosed as a waiter timeout. Keep context installation inside the child and add a focused regression proving the original replication exception reaches the caller.
Treat an absent source descriptor as a no-op when copying WebSocket context, matching the framework coroutine-context contract instead of reading an unchecked storage offset. Add a regression that preserves the destination context when the requested source has already disappeared.
Document captureFrom as the low-level keys-first API for taking a replicated coroutine context snapshot before a child exists. Clarify current and explicit source selection, transfer-only semantics, and the higher-level fork and copyContext alternatives. Update the general context guide to describe invocation-time snapshots accurately and record the Coroutine package's Hyperf upstream reference.
Capture the verified coroutine, context, lock-ownership, Waiter, and WebSocket findings with their final implementation, regression coverage, performance assessment, and review outcome. Mark the Coroutine package complete, route the next audit slice to Concurrency, and retain the cross-package revalidation links needed by the remaining package audits.
Encode successful serialized values as base64 so arbitrary binary results remain valid JSON across process boundaries. Build failure envelopes from structurally reconstructible constructor state, preserve falsey and floating values, contain reporting and encoding failures, and fall back to a stable RuntimeException response when the original exception cannot be transported safely. Add focused coverage for binary results, malformed state, constructor shapes, UTF-8 failures, reporter failures, and exact environment restoration.
Decode process responses with strict JSON and base64 handling, ignore appended gzip output, reconstruct transported failures from named constructor state, and contain invalid or non-Throwable parent-side classes as RuntimeException. Restore the current Laravel optional timeout contract across the facade, driver contract, and all implementations while applying the timeout only to process tasks, matching Laravel behavior. Cover binary results, malformed transport, reconstruction failures, falsey constructor values, timeout propagation, and unchanged coroutine and sync semantics.
Keep Testbench remote-closure decoding aligned with the framework process protocol: trim appended gzip bytes, decode JSON and base64 strictly, reconstruct named exception state, and contain invalid parent-side classes without masking the transported message. Document the full throwable boundary and add direct decoder coverage plus a real subprocess round trip for binary values.
Describe process-only timeouts and the exact copied-context contract, including shared ordinary object references and ReplicableContext behavior. Record the package provenance and material Hypervel differences, and mark Laravel fork-driver support as intentionally omitted because Swoole coroutines are the native execution model.
Capture the verified transport, reconstruction, timeout, documentation, and test-isolation findings together with the rejected complexity, owner-approved costs, validation, and final review outcome. Mark Concurrency complete, retain Foundation and Testbench revalidation dependencies, and route the next package audit to Coordinator.
Track the coroutine owned by each timer registration and distinguish framework-owned coordinator waits from user callbacks that are actively running or yielding. Clearing a timer can now release a blocked coroutine immediately without injecting cancellation into callback code. Guard coroutine ID publication against synchronous completion and creation-hook re-entry, preserve timer statistics across every exit, and declare Coordinator's direct Coroutine and PSR logger dependencies. Record the package's Hyperf provenance and the intentional clear behavior at the source boundary. Make coordinator concurrency tests failure-safe by signaling wait groups in finally blocks and asserting from the parent. Add deterministic regressions for clearing one or all timers, callback-yield safety, re-entry, self-clear, and spawn rollback, and update heartbeat tests for the new registration bookkeeping.
Introduce a narrow optional capability for frequency strategies that own resources and have Pool::close() invoke it before closing the channel and draining idle connections. ConstantFrequency participates through its existing clear method, so its timer and captured pool graph are released at the supported deterministic lifecycle boundary. Contain and report custom strategy cleanup failures through the existing pool reporter while continuing independent channel and connection teardown. This prevents an idempotent close from becoming permanently partial without expanding the general low-frequency contract or affecting borrow and release paths. Add regressions proving that closing a ConstantFrequency-backed pool releases its timer and stops later ticks, and that a throwing custom cleanup cannot prevent channel closure, connection cleanup, or bookkeeping reset.
Delete destructor cleanup from KeepaliveConnection, DbPool, and RedisPool. Their heartbeat callbacks retain the owning object while a timer is live, so the destructors cannot run when cleanup would be useful and only repeat a no-op after deterministic teardown has already completed. Keep explicit close paths as the sole resource owners: connection close clears keepalive state, while database and Redis pool close operations clear their heartbeat timers and their callbacks also stop during worker shutdown. No replacement garbage-collection machinery is necessary.
Document the verified Coordinator and Pool lifecycle findings, implemented ownership boundaries, rejected overengineered alternatives, regression coverage, performance and compatibility impact, validation, and final assessment. Mark Coordinator complete, route the next work unit to Signal, and carry the Pool, Database, and Redis teardown findings into the cross-package dependency index for revalidation during their later full audits.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates coroutine context capture and failure handling, process-driver transport and exception reconstruction, Swoole stream/WebSocket behavior, timer and pool teardown, filesystem lock ownership, and associated tests and documentation. It also removes obsolete HTTP portability and native HTTP overload surfaces. ChangesRuntime lifecycle and transport
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant ProcessDriver
participant Process
participant ProcessResult
Caller->>ProcessDriver: run(tasks, timeout)
ProcessDriver->>Process: apply timeout and execute task
Process-->>ProcessDriver: JSON envelope with base64 result
ProcessDriver->>ProcessResult: decode and reconstruct output
ProcessResult-->>Caller: value or reconstructed Throwable
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 hardens the low-level coroutine, process concurrency, engine I/O, coordinator timer, and pool teardown layers across the framework. The scope is wide (89 files) but the changes are logically coherent: each surface area gets a tightly targeted fix.
Confidence Score: 5/5Safe to merge — each fix is backed by a test and the rollback/teardown paths are carefully sequenced. No correctness regressions identified across the reviewed surfaces. Coroutine context capture is now transactional and validated by new tests. Process result decoding is stricter and fails gracefully. Timer cancellation correctly distinguishes the waiting state from a running callback. Destructor removal is justified by object-retention semantics. The only maintenance concern (duplicated decode logic across ProcessDriver and testbench ProcessResult) does not affect runtime correctness. src/testbench/src/Foundation/Process/ProcessResult.php contains a near-verbatim copy of the decode logic in ProcessDriver; the two should stay in sync. Important Files Changed
Reviews (2): Last reviewed commit: "Repair inline types in Markdown tables" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.md`:
- Line 617: Update the concurrency-04 table cell to escape or encode the pipes
in “CarbonInterval|int|null” so Markdown treats the union type as one cell and
MD056 passes; preserve the documented timeout API text and all other table
columns.
In `@src/concurrency/src/ProcessDriver.php`:
- Around line 72-101: Update the payload handling around the result-processing
logic to require a boolean successful field before branching, and validate that
the envelope contains the expected result data for successful responses.
Deserialize the decoded result with failure detection enabled, rejecting invalid
serialized data instead of treating unserialize() returning false as a
legitimate result while preserving valid false results.
In `@src/coordinator/README.md`:
- Line 6: Update src/coordinator/README.md:6 to add a “Differences From Laravel”
section explaining that Laravel has no direct coordinator equivalent and
documenting Hypervel’s coordinator behavior. Update src/coroutine/README.md:6
with the same required section, describing Hypervel’s Swoole coroutine model and
how it differs from Laravel.
In `@src/engine/README.md`:
- Around line 8-10: Move the Swoole-only portability note from the
“Architecture” section into a section headed exactly “Differences From Laravel”
in the README, preserving its existing content and meaning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 331fe680-275c-4755-a2be-955574e3f106
📒 Files selected for processing (87)
docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit.mddocs/plans/2026-07-12-test-suite-lifecycle-and-concurrency-robustness.mdsrc/boost/docs/concurrency.mdsrc/boost/docs/context.mdsrc/boost/docs/coroutine-context.mdsrc/concurrency/README.mdsrc/concurrency/src/ConcurrencyManager.phpsrc/concurrency/src/CoroutineDriver.phpsrc/concurrency/src/ProcessDriver.phpsrc/concurrency/src/SyncDriver.phpsrc/context/src/CoroutineContext.phpsrc/contracts/src/Concurrency/Driver.phpsrc/contracts/src/Engine/Http/ServerFactoryInterface.phpsrc/contracts/src/Engine/Http/ServerInterface.phpsrc/contracts/src/Engine/ResponseEmitterInterface.phpsrc/contracts/src/Engine/WebSocket/FrameInterface.phpsrc/coordinator/README.mdsrc/coordinator/composer.jsonsrc/coordinator/src/Timer.phpsrc/coroutine/README.mdsrc/coroutine/src/Channel/Caller.phpsrc/coroutine/src/Concurrent.phpsrc/coroutine/src/Coroutine.phpsrc/coroutine/src/Locker.phpsrc/coroutine/src/Mutex.phpsrc/coroutine/src/WaitConcurrent.phpsrc/coroutine/src/functions.phpsrc/database/src/Pool/DbPool.phpsrc/engine/README.mdsrc/engine/composer.jsonsrc/engine/src/EngineServiceProvider.phpsrc/engine/src/Http/EventStream.phpsrc/engine/src/Http/Server.phpsrc/engine/src/Http/ServerFactory.phpsrc/engine/src/Http/Stream.phpsrc/engine/src/Http/V2/Client.phpsrc/engine/src/ResponseEmitter.phpsrc/engine/src/WebSocket/Frame.phpsrc/engine/src/WebSocket/Response.phpsrc/engine/src/WebSocket/WebSocket.phpsrc/filesystem/src/Filesystem.phpsrc/filesystem/src/LockableFile.phpsrc/foundation/src/Console/InvokeSerializedClosureCommand.phpsrc/foundation/src/Testing/Coroutine/Waiter.phpsrc/log/src/Context/Repository.phpsrc/pool/src/ClearableFrequencyInterface.phpsrc/pool/src/ConstantFrequency.phpsrc/pool/src/KeepaliveConnection.phpsrc/pool/src/Pool.phpsrc/redis/src/Pool/RedisPool.phpsrc/support/src/Facades/Concurrency.phpsrc/testbench/src/Foundation/Process/ProcessResult.phpsrc/websocket-server/src/Context.phptests/Context/ContextCoroutineTest.phptests/Context/ContextTest.phptests/Context/Fixtures/ThrowingReplicableContext.phptests/Coordinator/CoordinatorManagerTest.phptests/Coordinator/CoordinatorTest.phptests/Coordinator/FunctionTest.phptests/Coordinator/TimerTest.phptests/Coroutine/Channel/CallerTest.phptests/Coroutine/ConcurrentForkTest.phptests/Coroutine/CoroutineCreateFailureTest.phptests/Coroutine/CoroutineNonCoroutineContextTest.phptests/Coroutine/CoroutineTest.phptests/Coroutine/LockerTest.phptests/Coroutine/MutexTest.phptests/Coroutine/ParallelTest.phptests/Coroutine/WaitConcurrentTest.phptests/Engine/Http2ClientTest.phptests/Engine/HttpServerTest.phptests/Engine/StreamTest.phptests/Engine/WebSocketTest.phptests/Filesystem/CoroutineLockOwnershipTest.phptests/Foundation/Console/Fixtures/ConcurrentProcessExceptionFixtures.phptests/Foundation/Console/InvokeSerializedClosureCommandTest.phptests/Foundation/Testing/Coroutine/WaiterTest.phptests/Integration/Concurrency/ConcurrencyTest.phptests/Integration/Database/Sqlite/DbPoolHeartbeatTest.phptests/Log/ContextCoroutineTest.phptests/Pool/HeartbeatConnectionTest.phptests/Pool/PoolTest.phptests/Redis/RedisPoolHeartbeatTest.phptests/Testbench/Foundation/Process/ProcessResultTest.phptests/Testbench/Foundation/Process/RemoteCommandTest.phptests/WebSocketServer/ContextTest.php
💤 Files with no reviewable changes (14)
- src/contracts/src/Engine/Http/ServerFactoryInterface.php
- src/engine/src/Http/Server.php
- src/contracts/src/Engine/Http/ServerInterface.php
- src/engine/src/EngineServiceProvider.php
- src/engine/composer.json
- src/engine/src/Http/ServerFactory.php
- src/contracts/src/Engine/ResponseEmitterInterface.php
- tests/Engine/HttpServerTest.php
- src/engine/src/Http/EventStream.php
- src/pool/src/KeepaliveConnection.php
- src/engine/src/ResponseEmitter.php
- src/database/src/Pool/DbPool.php
- src/redis/src/Pool/RedisPool.php
- src/coroutine/src/Concurrent.php
Validate the decoded response shape before either process consumer reads its success state or reconstructs a remote exception. Reject malformed failure metadata with stable consumer-specific errors instead of allowing protocol corruption to surface as unrelated type failures. Check unserialize failures at the native boundary so valid-base64 garbage cannot masquerade as a legitimate false result. Preserve real serialized false values and the existing arbitrary-object result contract. Add mirrored Concurrency and Testbench regressions for invalid envelope shapes, malformed failure fields, corrupt serialized bytes, and valid false round trips.
Escape literal union-type pipes in every affected plan table so GitHub's GFM table parser keeps each declaration in its intended cell. The escapes are source-only and render as normal pipe characters inside code spans. Apply the correction consistently across the object-pool plan, framework audit ledger, and enum-identifier plan rather than leaving the same rendering defect in adjacent rows.
|
@greptileai @coderabbitai Thanks for the reviews. I checked every inline and summary finding against the current contracts, upstream implementations, and call paths. Two corrections landed:
The remaining suggestions were intentionally left unchanged:
|
|
Tip For best results, initiate chat on the files or code changes.
The serialized-process envelope hardening and escaped GFM table pipes address the reported issues. The remaining decisions are also reasonable given the detached-stream contract, exception reconstruction safety requirements, package provenance, and existing documentation conventions. 🐇 ✅ |
|
The explanations are clear and well-grounded. A few notes:
Envelope validation — The hardened consumers with boolean Exception parameter JSON identity gate — The strict gate is the correct choice here. Once Coordinator/Coroutine Engine The two actionable findings are addressed and the intentional decisions are well-justified. LGTM. |
Summary
This change hardens the low-level runtime used for coroutine context, task creation, process concurrency, Engine I/O, coordinator timers, and pool teardown.
The main changes are:
For more details, see:
docs/plans/2026-07-12-framework-coroutine-state-lifecycle-audit-ledger.mdContext and coroutine lifecycle
CoroutineContextnow captures a complete snapshot before spawning or installing it elsewhere. Replication failure cannot leave partially copied state in a child, and callers can capture selected keys from the current coroutine or an explicit source coroutine through one documented API.Coroutine creation now treats bookkeeping as a transaction. Capacity tokens, waiter counts, caller replacements, mutex ownership, and forked-task tracking are restored when native creation fails. Forked tasks remain visible to their owner until all scheduled work is complete.
Filesystem locks now preserve coroutine ownership instead of allowing another coroutine sharing the same wrapper to unlock or replace an active lock.
Engine
The Engine package is now explicitly Swoole-only. The removed HTTP server factories, response emitter abstractions, and multi-engine stream wrappers were portability carryover from Hyperf and had no valid Hypervel consumer.
The remaining Swoole boundaries now:
This reduces the Engine surface while fixing real native-boundary behavior.
Process concurrency
Process results are transported without losing binary serialized payloads. Decoders reject malformed envelopes, tolerate appended gzip output where the subprocess protocol permits it, and contain parent-only class loading or constructor failures.
Exception reconstruction now preserves supported falsey and typed constructor values without reading inaccessible object state. Exceptions that cannot be reconstructed safely degrade to a
RuntimeExceptioncarrying the original message instead of causing a second parent-side failure.The public timeout option is carried through the concurrency contract and facade for Laravel parity. The Process driver applies it; Coroutine and Sync retain their existing execution behavior.
Coordinator and Pool teardown
Coordinator timers retain the exact coroutine ID they own and distinguish framework-owned waits from callbacks that are already running. Clearing a timer releases a coroutine blocked in Coordinator without injecting cancellation into user callback code.
Pool shutdown has a narrow optional capability for frequency strategies that own resources.
ConstantFrequencyuses its existingclear()method, and cleanup failures are reported without preventing channel closure or idle connection teardown.The destructors on
ConstantFrequency,KeepaliveConnection,DbPool, andRedisPoolwere removed. Their callbacks retain the owning object while live, so those destructors could not provide cleanup. Explicitclose()paths are now the sole lifecycle owners.Compatibility and performance
Laravel-facing APIs and existing call sites remain compatible. This restores Laravel process-timeout parity and adds documented Hypervel-specific context and teardown capabilities.
The removed Engine APIs were unused Hypervel-specific multi-server abstractions. Hypervel supports Swoole only.
There is no new normal request or job overhead. Added work is limited to the operation that needs it:
No polling loop, retry framework, generic cancellation registry, reflection-based lifecycle detection, or per-timer channel was introduced.
Testing
The regression coverage exercises:
The full formatter, static-analysis, parallel test, Testbench contract, and Testbench dogfood gates pass.
Summary by CodeRabbit
processdriver (other drivers accept the parameter for compatibility).processtask result transport, including lossless binary payload support.