Skip to content

Make Guzzle coroutine-safe and reduce AOP overhead - #656

Merged
binaryfire merged 21 commits into
0.4from
fix/guzzle-coroutine-ownership
Oct 9, 2026
Merged

binaryfire merged 21 commits into
0.4from
fix/guzzle-coroutine-ownership

Conversation

@binaryfire

@binaryfire binaryfire commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Guzzle's process-wide promise queue can run one request's callbacks while another request drains it. In a long-lived coroutine worker, those callbacks can resolve the wrong request or tenant, or use a database transaction owned by another coroutine. Sharing an active cURL multi-handler has a second failure mode: one coroutine can drive another's native transfers and crash the worker.

This PR makes those operations stay with their owning coroutine, fixes the affected SDK consumers, and reduces the AOP cost of enforcing that rule. The same AOP improvements benefit existing instrumentation such as Sentry and Telescope.

Keep Guzzle callbacks and transfers in their owning coroutine

A mutable promise belongs to the native coroutine that creates it. Pending work must be registered, driven, settled and canceled there. Completed results remain reusable across coroutines, and an idle multi-handler can be reused sequentially. A promise that has adopted another pending promise is still unfinished work.

Each coroutine receives a normal Guzzle task queue through the supported Utils::queue() setter. Pending promise operations and active multi-handlers are checked before another coroutine can run their callbacks or change their state. A conflicting operation fails with an explanation of how to create and finish the work in the same coroutine. Applications retain concurrency through native coroutines and parallel(); they share completed results rather than handing pending operations between requests.

Transfer ownership is reserved before request preparation, because reading an upload body can yield. Active transfers remain alive until settlement or cancellation. At coroutine exit, unfinished transfers are canceled and the owner's queue drains their rejection handlers, allowing dependent promises and memoized providers to recover. Queue and transfer state is excluded from copied coroutine context. Outside coroutines, normal Guzzle queue and shutdown behavior is retained.

Registration belongs to the HTTP provider, so protection applies in HTTP requests, queue workers and console commands without requiring an observability package. It covers Guzzle's mutable promises and cURL multi-handler; arbitrary third-party promise implementations and custom transports do not acquire these guarantees automatically.

Why AOP is needed

Guzzle exposes a queue replacement hook, but no global hook for promise construction, pending promise operations, or multi-handler admission and driving. An application client subclass or outer middleware cannot intercept every promise created inside Guzzle or an SDK.

The aspects therefore target a small set of public methods: the mutable promise constructor, then, wait, cancel, resolve and reject; and the multi-handler's __invoke, tick, execute and close where available. They check ownership around the original methods without copying their implementations or replacing Guzzle's scheduling.

Proxy generation runs before provider boot. It rejects vendor classes loaded too early and loaded proxies missing required method interception, rather than silently leaving protection inactive. The error explains lazy client registration. Generated method metadata verifies coverage during bootstrap, without adding that inspection to each invocation.

Correct shared SDK and HTTP consumers

AWS credentials. SQS, S3 and SES clients can share a callable credential provider even when the clients themselves are pooled separately. The provider can retain an in-flight HTTP promise. A shared wrapper serializes provider invocation and its promise completion by callable identity, while each caller executes in its own coroutine and receives its own result or exception. Callbacks drained during that wait may request credentials again in the same coroutine without deadlocking; other coroutines still wait for the outermost call to finish. Existing SDK memoization, pool fingerprints, completed credential reuse and cancellation behavior are preserved. There is no additional credential cache or sharing of one caller's credentials with another.

Pusher and Reverb. The default client previously shared an async multi-handler between concurrent callers. Async handlers are now reused within the same client and coroutine, preserving same-coroutine fan-out and connection reuse. Weak client keys and coroutine-local storage release handlers when they are no longer needed. Synchronous transport reuse, middleware, timeouts, on_stats, custom handlers and supported transport options remain intact. Connection caps that require sharing one active multi-handler across coroutines are rejected; callers should bound coroutine concurrency or use rate limiting.

Named HTTP connections. Async requests previously lost the named connection's handler settings when creating a fresh handler. Factory::newConnectionHandler() now shares option derivation with the cached synchronous path. Async handlers remain isolated while retaining their registered transport settings. Per-call request presets and explicit handler/client overrides keep their existing meaning.

Reduce shared AOP work

The previous dispatcher rebuilt a generic pipeline for every intercepted call and reflected its closure to find the target instance. It now caches an immutable chain per class and method, containing only aspect class names and continuation closures. Each invocation still resolves its aspect from the current container. Generated proxies pass the intercepted instance directly, or null for static calls.

This preserves priority, short-circuiting, original-method bypass, exceptions, references, variadics, scoped aspects, container rebinding, and nested or concurrent calls. No target object, join point, container or aspect instance is retained in the chain. The unused AOP-specific pipeline and its dependency are removed; the general framework pipeline is unchanged.

Bootstrap now selects exact class rules directly, prepares wildcard matching once per rule, and creates the parser and printer only when generation is required. A separate authoritative Composer loader contains only proxy entries, keeping the application's source map unchanged. This avoids retaining copies of the complete application map. Composer discovery continues to identify the application loader, including after test resets. Content fingerprints, PSR-4 lookup, source overrides, proxy regeneration and validation before publication remain intact.

Make class-map overrides safe across application boots

The documented provider classMap() API previously copied the entire Composer application map when adding replacements. It also rejected the same replacement on a second Testbench boot, while test cleanup left unloaded overrides active for later applications that had not registered them.

Replacements now use their own authoritative Composer loader and leave the application map unchanged. Automatic test cleanup unregisters that loader and its generated replacement proxies together. A loaded target can be registered again only from the same canonical source; generated proxies record their original source as class metadata so the same rule applies to them. Different-source replacements still fail. Replacement proxies use separate, fixed filenames so they cannot overwrite ordinary proxies retained between application boots.

Generated proxies now live in bootstrap/cache/aop, alongside each release's other compiled framework files. Two overlapping releases sharing storage previously wrote the same proxy paths, allowing an older worker to load the newer release's code. The release-local directory prevents that collision without per-release hashes or accumulating files in shared storage. Cache clearing, Testbench cleanup and the about report follow the new location; about also recognizes the generated PHP files correctly.

Approaches considered and rejected

Approach Why it was not used
Replace only Guzzle's queue Necessary, but insufficient: another coroutine could still operate directly on a pending promise or active multi-handler.
Client subclasses or outer middleware alone Do not cover promises and callbacks constructed internally by Guzzle and third-party SDKs.
Copy request context into another coroutine Copying values does not transfer native resource or database transaction ownership.
Replace the scheduler or make requests pump each other's queues Adds scheduling machinery and changes wait/progress behavior. Keeping work in its native owner preserves the relevant resource boundary.
Maintain a Guzzle fork, vendor patch, or copied promise implementation Duplicates dependency maintenance. Supported queue replacement plus narrow public-method interception provides the required coverage.
Give every Pusher transfer its own handler Loses supported same-coroutine fan-out and connection reuse. Per-client, per-coroutine reuse preserves both.
Cache resolved aspects or coroutine state globally Would break scoped bindings, rebinding, or request isolation. Only immutable dispatch metadata is cached.
Publish proxies into the main Composer map Retains another complete class-map allocation. Composer's own separate authoritative loader holds just the proxy entries.

The pass-through-aspect variant used below is a measurement control, not a production bypass. All measured outgoing request paths remain supported; the rejected items above are implementation approaches.

AOP performance improvements

These comparisons are against the initial ownership implementation, not against an uninstrumented framework:

Measurement Initial implementation Optimized Change
Guarded promise-chain CPU 39.52 µs/op 20.57 µs/op About 48% lower
Cached startup 181.16 ms 78.15 ms About 57% lower
Retained PHP heap after idle workload 6,114.61 KiB 3,164.52 KiB 2.88 MiB less

The compiled-chain comparison reduced pass-through dispatch from 31.62 to 15.80 µs/op. A separate comparison isolated direct instance passing, reducing guarded chain CPU from 21.72 to 20.48 µs/op. The earlier complete matrix measured 20.50 µs/op; the final bootstrap refresh measured 20.57 with the same dispatch code.

Ownership overhead across outgoing paths

The following compares the unchanged framework with the protected implementation. Values are client CPU microseconds per operation, not batch latency. The loopback origin's CPU is excluded.

Workload Concurrency Baseline → protected Added CPU
Idle loop † 1 0.33 → 0.34 0.01, rounding-sensitive
Promise chain † 1 3.23 → 20.57 17.34
Synchronous HTTP † 1 405.37 → 444.67 39.30
Async HTTP 1 289.60 → 377.82 88.22
Async HTTP 8 200.12 → 259.75 59.63
Async HTTP † 32 152.42 → 210.28 57.86
Coroutine-parallel HTTP 1 466.42 → 519.82 53.40
Coroutine-parallel HTTP 8 422.48 → 482.29 59.80
Coroutine-parallel HTTP † 32 430.56 → 468.59 38.03
Response middleware/event 1 301.30 → 339.90 38.61
Credential provider 1 371.19 → 419.12 47.94
Shared credential provider 8 294.69 → 409.29 114.60
Pusher 1 390.30 → 500.75 110.45
Pusher 8 299.13 → 374.29 75.16
Pusher † 32 267.77 → 330.34 62.57
Cancel pending transfer † 1 108.96 → 165.89 56.93

† Refreshed paired measurements after the bootstrap corrections. Other rows use the earlier complete matrix with the same dispatch and ownership implementation. Rows from different runs are not a controlled concurrency-scaling comparison.

Async completion through Utils::unwrap() makes 27 intercepted calls per operation at concurrency one, compared with nine in the promise probe. A separate pass-through comparison separates dispatch from ownership and queue handling; it explains the larger async and cancellation costs. There were no GC runs inside the timed operation loops.

The shared credential-provider workload deliberately serializes calls to one provider and includes a 1 ms endpoint delay. At concurrency eight, p95 latency increased from 5.396 to 22.474 ms. This includes waiting for the provider lock; it is not all AOP overhead. Completed memoized credentials remain reusable.

Guzzle 7 shows comparable absolute costs on the focused workloads: approximately 17 µs for promise chains, 43 µs for synchronous HTTP, 90 µs for async concurrency one, 53 µs for async concurrency eight, and 54 µs for cancellation. The source implementation does not need a separate version-specific AOP path.

Memory bounds and capacity planning

At the benchmarked revision, the integration retains approximately 124–188 KiB of additional PHP heap per warmed worker across the measured workloads, with roughly 0.94–1.19 MiB additional RSS. RSS includes shared code pages, so it is not a measure of private memory added to every worker. Allocated PHP memory after exit remained 16 → 16 MiB.

This retained metadata is not added for each request. Temporary ownership state follows outstanding promises and native transfers and is released as work finishes or is canceled. Repeated batches showed the same report-bookkeeping growth on baseline and protected processes, with no additional retained growth, and returned to the same descriptor count. Applications still need bounded outgoing concurrency and queues; the integration does not cap how much unfinished work an application can create.

At the measured 38.03 µs additional CPU per outgoing request for coroutine-parallel HTTP at concurrency 32:

Outgoing requests/sec Additional CPU capacity across the deployment
1,000 0.038 cores
10,000 0.380 cores
50,000 1.902 cores

These are arithmetic projections, not throughput measurements at those traffic levels. The tested HTTP and Pusher paths suggest a planning range of approximately 40–110 µs/request, or 0.4–1.1 additional cores at 10,000 outgoing requests/sec. Credential-provider contention is a separate serialized workload. The integration adds predictable CPU work without a request-count-dependent retained-memory cost.

Cached startup compared with the unchanged framework is 69.95 → 78.15 ms. One cold process requiring proxy generation took 132.45 ms; cold startup is reported separately from repeated warm measurements.

Measurement conditions and validation

Measurements precede the subsequent class-map override reset and release-local proxy corrections, and the switch to xxh128 for internal proxy fingerprints and helper names. These add generated source metadata and bootstrap lookup for registered overrides, without changing the per-request dispatcher or ownership checks. Their startup and retained-memory effects have not been remeasured; the resource figures describe the benchmarked revision.

The credential-provider measurements also precede the same-coroutine reentry correction. Its active-owner lookup and cleanup have not been remeasured. This correction does not change the HTTP, Pusher or shared AOP dispatch paths.

Measurements used PHP 8.4.25, Swoole 6.2.2, cURL 8.5.0, CLI OPcache enabled, JIT disabled, and an Intel Core i7-7700 with six logical processors exposed. Both checkouts used independently installed identical dependencies and optimized Composer autoloading. The primary matrix used Guzzle 8.2.0/promises 3.0.2; focused comparisons also used Guzzle 7.15.5/promises 2.5.3. Each workload used three alternating process pairs with warmup and three measured samples per process. The report includes ranges, operation counts, latency and resource snapshots.

Async/Pusher wall-time measurements at higher concurrency include the separate native hooked-select readiness stall addressed by Swoole #6290. CPU is reported separately rather than using that stall to make ownership overhead appear negligible. The default AWS Guzzle transport also takes the async transport path for synchronous SDK calls.

Regression coverage exercises real generated proxies, yielding request/tenant and database-transaction isolation, pending and adopted promises, cancellation, owner exit, upload preparation, shared clients, credential memoization, Pusher fan-out, cold/cached bootstrap and cleanup. Representative tests fail with the protections removed. Both supported Guzzle dependency families and their upstream promise suites were validated, together with the full framework suite, affected SDK consumers, Sentry/Telescope, package-mode Testbench, formatting and static analysis. CI adds a focused Guzzle 7 compatibility job alongside the normal dependency resolution.

The shared PHPUnit extension prepares framework proxy targets before test discovery without enabling optional instrumentation and resets test-owned queues safely. The formatter's return_assignment rule is also disabled because it removes assignments captured by reference in nested closures, breaking valid Guzzle wait callbacks.

Validation also exposed test-harness gaps. Coroutine tests now retain PHPUnit's native busy-loop alarm and enforce the same deadline while the test or its children are suspended, reporting the named test through PHPUnit's normal timeout result. Synchronous output-assertion tests run outside coroutines pending the upstream output-buffer integration. PHP-clock rate-limiter contract tests freeze their clock so I/O delays cannot change back-to-back decisions.

Notification transport snapshots also use SplObjectStorage::offsetExists() instead of the PHP 8.5-deprecated contains(), retaining PHP 8.4 compatibility.

The technical design is in docs/plans/2026-10-09-guzzle-coroutine-ownership.md. Reproducible commands and complete measurements are in tests/Benchmarks/GuzzleOwnership/README.md and results.md.

Note

Make Guzzle coroutine-safe and reduce AOP dispatch overhead

  • Adds coroutine ownership tracking for Guzzle promises and cURL multi-handlers. Pending promises and active transfers belong to their creating coroutine; foreign operations raise CoroutineOwnershipException (CoroutineOwnership.php, Aspects/PromiseOperationAspect.php).
  • Adds CoroutineTaskQueue, CoroutineState, and TransportState so each coroutine has its own Guzzle task queue and pending transfers. Coroutine exit cancels unfinished transfers and drains that coroutine's callbacks (CoroutineTaskQueue.php).
  • Rewrites AOP dispatch: AspectManager caches compiled closure chains instead of aspect-name arrays, ProxyDispatcher invokes cached chains with the intercepted instance attached to the join point (AspectManager.php, ProxyDispatcher.php).
  • Wraps callable AWS credential providers in SerializedCredentialProvider for SQS, S3, SES mail, and filesystem clients, so shared providers serialize their work per caller. Adds shared Pusher/Reverb transports for synchronous calls and coroutine-local handlers for asynchronous calls (BroadcastManager.php).
  • Moves the AOP proxy cache from storage/framework/aop to bootstrap/cache/aop, switches generated fingerprints and helper names to xxh128, and adds ProxySource/ProxyMethod attributes to generated proxies. Adds a CI job for Guzzle 7 compatibility.
  • Behavioral Change: Pusher/Reverb clients without an explicit handler now throw InvalidArgumentException for host/total connection-limit options; ProceedingJoinPoint::getInstance now reports the dispatcher-supplied instance (null for static calls) instead of reflecting the closure; cached AOP entries are closures, so custom AspectManager consumers must be updated; existing proxy caches under storage/framework/aop are stale and regenerate under the new path.

Macroscope summarized 1e7f66f.

The formatter removes assignments captured by reference in nested closures, breaking Guzzle wait callbacks. Explicitly disable return_assignment rather than changing valid code to avoid the rewrite. Verified with the failing formatter reproduction, a clean formatting run and the credential-provider regression tests.
SQS, S3 and SES clients can share a custom credential provider across pools. Serialize provider invocation and promise completion using its callable identity so concurrent callers cannot drive each other’s pending credential work or receive another caller’s credentials. Preserve SDK memoization, original pool identity and cancellation cleanup without adding a credential cache.

Cover actual pooled manager paths, caller-specific request signing, callable aliases, expiring credential reuse, failure and cancellation. Document safe memoization for providers whose callers share credentials. Verified focused regressions, Queue, Filesystem and Mail suites, full static analysis and formatting.
Pusher and Reverb shared one Guzzle multi-handler across concurrent callers, allowing one coroutine to drive another’s native transfers. Reuse async handlers only within the same client and coroutine, using weak client keys and non-copyable context. Preserve same-coroutine promise fan-out and connection reuse alongside the existing synchronous transport.

Keep custom-handler validation and transport options intact. Reject connection caps that require an unsafe cross-coroutine shared handler and document bounded concurrency alternatives. Regression tests reproduce the native failure, preserve overlapping requests, and verify copied-context isolation, option forwarding and connection reuse. Broadcasting tests, static analysis and formatting pass.
Async requests created fresh handlers without the named connection's transport
settings. Add an uncached handler factory that shares option derivation with
the synchronous path, preserving transport sharing and multiplexing policy
while keeping each async handler isolated.

Cover sync and async option forwarding, request-preset replacement, and handler
reuse. Connection tests and affected consumers pass on Guzzle 7 and 8; static
analysis and formatting pass.
Guzzle's global task queue can run one request's callbacks while another
request drains it. Shared multi-handlers can also drive foreign transfers.
Install coroutine-local stock queues and guard pending promise operations
and active transports at their public method boundaries. Guzzle has no global
hooks for these boundaries, so use narrowly targeted AOP interception.

Retain active native transfers until settlement and propagate cancellation
through the owner's queue at exit. Completed results and idle handlers remain
reusable. Reject classes loaded before proxy generation or with stale method
coverage, and prepare framework proxies before PHPUnit test discovery without
activating optional instrumentation.

Cover ownership, retries, cancellation, memoized credential recovery, context
copying, shared synchronous clients, bootstrap ordering and resource cleanup.
Fix whole-class aspect matching so explicit constructor interception does not
activate unrelated class-wide aspects. Document ownership and async transport
configuration behavior.

Validated on Guzzle 8/promises 3 and Guzzle 7/promises 2, including regression
mutations. HTTP, DI, testing, Sentry, Telescope, Testbench package-mode and
affected AWS/broadcasting checks pass, along with analysis and formatting.
Remove each transfer from the coroutine holder before canceling it. Exit
cleanup now makes progress through its own collection instead of depending
on the promise aspect to remove the entry indirectly. Normal settlement
still releases transport ownership through the existing bookkeeping.

The ownership, exit cleanup and retry tests pass on Guzzle 7 and 8.
Add focused Guzzle 7 CI coverage, including streaming transports, and a manual runner for unchanged upstream promise suites. Consolidate yielding callback coverage around real request and database transaction isolation.

Exercise Saloon Digest authentication over a loopback connection so its test works with both native cURL negotiation and Guzzle authentication middleware.

Validation: full framework suites passed on Guzzle 8 and 7; both upstream promise suites passed with and without the integration. Expanded compatibility checks and the final consolidated tests pass on their supported versions. A stock-queue mutation reproduces the cross-request context failure.
Add a developer harness using production provider registration and generated proxies, with a separate loopback origin. Measure startup independently from promise, HTTP, middleware, credential, broadcasting and cancellation workloads, including process CPU, memory, descriptors and cleanup.

Document paired baseline comparisons, named connection reuse, the diagnostic pass-through aspect and measurement limits. Benchmark PHP files pass syntax checks and formatting.
Avoid rebuilding the generic aspect pipeline and reflecting the original closure for every intercepted call. Cache only immutable method chains, while resolving each aspect from the current container and keeping all invocation state local.

Preserve priority, rebinding, scoped aspects, nested and concurrent dispatch, short-circuiting, references, and cleanup. Remove the unused AOP pipeline and its direct package dependency.

Validated with focused AOP and consumer tests, both Guzzle dependency families, static analysis, formatting, the full 44,038-test suite, and package-mode Testbench. Measured guarded promise-chain CPU drops by about 48% from the initial implementation.
Publish generated proxies through a separate authoritative Composer loader so the application class map remains unchanged. Resolve the application loader through Composer registration metadata and preserve source lookup, regeneration, and validation before publication.

Select exact targets directly, compile wildcard patterns once, enumerate the fixed generator directory without Finder, and construct the parser and printer only when code generation is required. These changes remove repeated map scanning and two retained full-map copies across the initial implementation, while preserving proxy fingerprints and visitor behavior.

Validated cold and cached startup, PSR-4 sources, overrides, atomic failures, and repeated applications. Both dependency families pass focused coverage; the full suite passes 44,038 tests, package-mode Testbench passes 635 tests, and the package consumer fixture passes 6 tests. Measured retained heap falls by 2.88 MiB and cached bootstrap time by about 57% versus the initial ownership implementation.
Record paired Guzzle 7 and 8 measurements, shared dispatcher improvements, cold and cached bootstrap, retained heap and RSS, cancellation, and repeated-batch cleanup. Identify the native Swoole select delay separately and distinguish refreshed measurements from earlier runs.

Include the profile-backed explanation of remaining dispatch, queue, and startup costs. Report autoloader configuration in the reusable harness and release its measured closure before retained-memory snapshots. Keep raw reports and temporary investigation artifacts outside the repository.

Full framework, package-mode Testbench, package consumer, focused compatibility, formatting, and static-analysis checks passed.
Translate measured per-request CPU overhead into deployment-wide core estimates at example traffic levels, distinguishing arithmetic projections from throughput measurements. Explain bounded worker metadata and temporary in-flight tracking, and identify the measured outgoing paths separately from serialized credential contention.

Use public reproducibility instructions in the benchmark guide and identify the published implementation revision. Benchmark syntax checks and the latest base streaming tests pass; no timing measurements were rerun.
Preserve the technical plan: failure modes, ownership guarantees, bootstrap and cleanup, SDK consumers, supported-version coverage, and benchmark methodology. Link the measured results and retain the reasons for the chosen boundaries without implementation coordination or progress records.
PHPUnit captures output outside the automatic test coroutine, allowing empty-output expectations to pass without observing printed content. Keep the environment bootstrap and response factory tests in the native output-capture context and assert the streamed cancellation output directly.

Document the current limitation and track the integration work for PHPUnit issue #7039. Both affected test classes and the focused formatting check pass.
Record upstream PR #7040 alongside issue #7039 so the coroutine test integration follow-up can track the proposed fix.
Freeze the PHP clock when constructing the SQLite, Swoole and worker-array contract limiters. Elapsed I/O and scheduling time must not change consecutive decisions that the shared contract expects to occur at one instant. Keep the existing clock advancement and global cleanup behavior.

Validated the affected store tests, the delayed SQLite reproduction, and the full framework suite.
Keep the native alarm for non-yielding work and observe root completion through a private buffered channel. Continue checking remaining children against the same deadline without taking the join slots used by the tested code. Route expiry through the native PHPUnit timeout result and cancel blocked children without throwing into raw callbacks.

Cover busy loops, sleeping tests, abandoned children, normal root and child joins, and disabled limits. Preserve coroutine-count assertions relative to their initial infrastructure state. Full framework, Testbench, dogfood, formatting and static-analysis checks pass. A temporary path count verified that after-root polling is rare; no diagnostic code is retained.
Use a separate authoritative loader for source overrides and their generated proxies so application maps are not copied or polluted. Reset unloaded overrides between application boots, accept loaded replacements only from the same canonical source, and record original proxy source paths as generated metadata. Separate replacement filenames preserve ordinary proxies retained by the persistent loader.

Generate proxies beneath bootstrap/cache/aop so overlapping releases sharing storage cannot overwrite each other. Update cache clearing, the about report, Testbench cleanup and documentation. Use xxh128 for internal fingerprints and helper names, retaining source-path, content and rule invalidation coverage.

Validate repeated boot, cleanup, replacement priority, source regeneration and shared-storage releases. Full framework, Testbench, dogfood, formatting and analysis pass. Preserve the benchmark revision and explicitly identify later unmeasured bootstrap corrections in the report; runtime dispatch and ownership checks are unchanged.
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: hypervel/components/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cf185aae-05c2-4d67-b720-088b9d955ffa

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:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@binaryfire

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review skipped: 108 files exceed the limit of 100.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@binaryfire

Copy link
Copy Markdown
Member Author

@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 9, 2026

Copy link
Copy Markdown

@cubic-dev-ai review

@binaryfire cubic can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 124,545 of the 120,000 allowed lines of code this month. Reviews resume on 10 October 2026 (in 1 day). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Make Guzzle coroutine-safe and reduce AOP overhead

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Keep pending Guzzle promises, callbacks, and cURL transfers in their owning coroutine.
• Fix concurrent AWS credential, Pusher, and named HTTP connection usage.
• Cache AOP dispatch chains and isolate proxies from application class maps and shared storage.
• Add regression tests, Guzzle 7 CI, benchmarks, and usage guidance.
Diagram

graph TD
  Provider["HTTP provider"] --> Queue["Coroutine task queue"] --> State["Coroutine state"]
  Provider --> Aspects["Ownership aspects"] --> Registry["Ownership registry"]
  Aspects --> Dispatcher["AOP dispatcher"] --> Chains["Cached aspect chains"]
  Registry --> State
  Consumers["SDK consumers"] --> Aspects
Loading
High-Level Assessment

Keep Guzzle's supported queue replacement alongside narrow public-method AOP interception. Replacing the queue alone cannot prevent direct foreign operations on pending promises or active multi-handlers; client middleware cannot intercept work created inside Guzzle or SDKs. Caching dispatch metadata rather than resolved aspect instances preserves scoped bindings.

Files changed (106) +4972 / -424

Enhancement (13) +517 / -49
PusherHandlerContext.phpStore Pusher handlers in noncopyable context +26/-0

Store Pusher handlers in noncopyable context

• Adds coroutine-local, weakly keyed storage for asynchronous handlers.

src/broadcasting/src/PusherHandlerContext.php

Ast.phpInitialize parser and printer lazily +6/-9

Initialize parser and printer lazily

• Avoids constructing PHP parsing machinery unless proxy generation requires it.

src/di/src/Aop/Ast.php

ProxyCallVisitor.phpEmit instance and proxy metadata +12/-1

Emit instance and proxy metadata

• Generated methods pass their instance directly to dispatch and record original-source and helper-method attributes.

src/di/src/Aop/ProxyCallVisitor.php

ProxyManager.phpOptimize proxy matching and separate replacements +26/-39

Optimize proxy matching and separate replacements

• Selects exact rules directly, prepares wildcard matching per rule, uses xxh128 fingerprints, and gives replacement proxies distinct filenames.

src/di/src/Aop/ProxyManager.php

ProxyMethod.phpMark intercepted proxy methods +18/-0

Mark intercepted proxy methods

• Adds an attribute identifying each generated method's original-body helper for bootstrap validation.

src/di/src/Aop/ProxyMethod.php

ProxySource.phpRecord original proxy source +18/-0

Record original proxy source

• Adds class metadata used to validate repeat registrations of loaded replacements.

src/di/src/Aop/ProxySource.php

PromiseConstructionAspect.phpRecord new promise owners +26/-0

Record new promise owners

• Intercepts successful mutable promise construction and records the native coroutine.

src/http/src/Client/Guzzle/Aspects/PromiseConstructionAspect.php

CoroutineOwnership.phpTrack promise and transport ownership +162/-0

Track promise and transport ownership

• Uses weak maps to reject foreign operations on pending promises or reserved multi-handlers while allowing completed-result and idle-handler reuse.

src/http/src/Client/Guzzle/CoroutineOwnership.php

CoroutineState.phpClean up owner-local pending work +101/-0

Clean up owner-local pending work

• Holds a noncopyable coroutine queue and unfinished transfers; cancels transfers and drains callbacks at exit.

src/http/src/Client/Guzzle/CoroutineState.php

CoroutineTaskQueue.phpRoute Guzzle tasks by coroutine +83/-0

Route Guzzle tasks by coroutine

• Installs through Guzzle's queue setter while preserving the existing queue outside coroutines.

src/http/src/Client/Guzzle/CoroutineTaskQueue.php

TransportState.phpCount multi-handler reservations +17/-0

Count multi-handler reservations

• Stores the owning coroutine and number of active transfers or preparations.

src/http/src/Client/Guzzle/TransportState.php

CoroutineOwnershipException.phpDefine ownership violation exception +11/-0

Define ownership violation exception

• Introduces a dedicated logic exception for foreign pending-promise and active-transport operations.

src/http/src/Exceptions/CoroutineOwnershipException.php

HttpServiceProvider.phpInstall ownership protection in HTTP provider +11/-0

Install ownership protection in HTTP provider

• Registers the Guzzle queue and promise/transport aspects independently of optional observability providers.

src/http/src/HttpServiceProvider.php

Bug fix (15) +507 / -90
BroadcastManager.phpIsolate asynchronous Pusher handlers +51/-9

Isolate asynchronous Pusher handlers

• Reuses handlers within each client and coroutine while retaining synchronous reuse. Rejects connection caps requiring unsafe active-handler sharing.

src/broadcasting/src/BroadcastManager.php

Aspect.phpPreserve explicit constructor interception +5/-1

Preserve explicit constructor interception

• Prevents class-wide rules from implicitly including constructors while retaining explicit method rules.

src/di/src/Aop/Aspect.php

GenerateProxies.phpPublish and validate release-local proxies +105/-46

Publish and validate release-local proxies

• Writes proxies under bootstrap/cache/aop using a separate authoritative loader. Rejects prematurely loaded targets and loaded proxies lacking required interception.

src/di/src/Bootstrap/GenerateProxies.php

ClassMapManager.phpIsolate replacements from the application loader +38/-7

Isolate replacements from the application loader

• Uses an unregisterable authoritative loader for overrides and their proxies. Accepts a loaded target only when its canonical source matches.

src/di/src/ClassMap/ClassMapManager.php

FilesystemManager.phpProtect callable S3 credentials +5/-0

Protect callable S3 credentials

• Wraps S3 credential callbacks in the shared-provider serializer before constructing clients.

src/filesystem/src/FilesystemManager.php

AboutCommand.phpReport release-local AOP proxies +1/-1

Report release-local AOP proxies

• Finds generated PHP files under bootstrap/cache/aop without incorrectly treating them as framework cache files.

src/foundation/src/Console/AboutCommand.php

RunTestsInCoroutine.phpEnforce timeouts during suspended coroutine tests +91/-8

Enforce timeouts during suspended coroutine tests

• Keeps PHPUnit's native alarm and adds a coroutine-aware deadline that cancels stalled tests and children.

src/foundation/src/Testing/Concerns/RunTestsInCoroutine.php

Factory.phpShare named-handler option derivation +9/-7

Share named-handler option derivation

• Introduces newConnectionHandler() to create isolated handlers using registered transport settings.

src/http/src/Client/Factory.php

PromiseOperationAspect.phpGuard pending promise operations +55/-0

Guard pending promise operations

• Checks ownership around continuation, waiting, cancellation, and settlement; tracks adopted pending promises.

src/http/src/Client/Guzzle/Aspects/PromiseOperationAspect.php

TransportOwnershipAspect.phpGuard active cURL multi-handlers +56/-0

Guard active cURL multi-handlers

• Reserves ownership before request preparation and tracks or releases transfers on success, failure, and settlement.

src/http/src/Client/Guzzle/Aspects/TransportOwnershipAspect.php

PendingRequest.phpPreserve asynchronous named-connection options +4/-2

Preserve asynchronous named-connection options

• Uses a fresh registered-options handler for asynchronous requests while retaining the cached synchronous handler.

src/http/src/Client/PendingRequest.php

MailManager.phpProtect callable SES credentials +7/-2

Protect callable SES credentials

• Wraps configured SES v2 credential callbacks before client construction.

src/mail/src/MailManager.php

SqsConnector.phpProtect callable SQS credentials +2/-1

Protect callable SQS credentials

• Serializes resolved callable providers without changing noncallable or configured-provider handling.

src/queue/src/Connectors/SqsConnector.php

SerializedCredentialProvider.phpSerialize shared credential resolution +74/-0

Serialize shared credential resolution

• Locks by callable identity through provider invocation and promise completion, returning each caller's own result or exception.

src/support/src/Aws/SerializedCredentialProvider.php

Composer.phpDiscover the application Composer loader +4/-6

Discover the application Composer loader

• Uses Composer's registered vendor loaders instead of selecting a prepended auxiliary proxy loader.

src/support/src/Composer.php

Refactor (6) +85 / -79
AspectCollector.phpCentralize aspect registration +29/-0

Centralize aspect registration

• Adds registration using reflected default targeting rules and priority.

src/di/src/Aop/AspectCollector.php

AspectManager.phpCache compiled aspect chains +16/-21

Cache compiled aspect chains

• Stores immutable continuation closures per class and method instead of aspect lists.

src/di/src/Aop/AspectManager.php

ProceedingJoinPoint.phpAccept the intercepted instance directly +8/-7

Accept the intercepted instance directly

• Removes per-invocation reflection of the original closure to discover its bound object.

src/di/src/Aop/ProceedingJoinPoint.php

ProxyDispatcher.phpReplace per-call pipeline with compiled dispatch +25/-24

Replace per-call pipeline with compiled dispatch

• Compiles ordered aspect continuations once per method while resolving each aspect from the current container on invocation.

src/di/src/Aop/ProxyDispatcher.php

RewriteCollection.phpCentralize class-rule exclusions +5/-10

Centralize class-rule exclusions

• Defines constructor exclusion for class-wide rules while permitting explicit interception.

src/di/src/Aop/RewriteCollection.php

ServiceProvider.phpDelegate aspect-default registration +2/-17

Delegate aspect-default registration

• Uses AspectCollector for reflected aspect rules and clarifies repeated class-map overrides.

src/support/src/ServiceProvider.php

Documentation (17) +471 / -6
2026-10-09-guzzle-coroutine-ownership.mdDocument the ownership design +190/-0

Document the ownership design

• Adds the technical design and rationale for coroutine-safe Guzzle integration.

docs/plans/2026-10-09-guzzle-coroutine-ownership.md

todo.mdTrack PHPUnit output-buffer follow-up +1/-0

Track PHPUnit output-buffer follow-up

• Records the upstream output-buffer work needed for coroutine output assertions.

docs/todo.md

README.mdUpdate broadcasting package guidance +2/-0

Update broadcasting package guidance

• Points package users to coroutine-safe Pusher transport behavior.

src/broadcasting/README.md

aop.mdDocument proxy generation requirements +6/-2

Document proxy generation requirements

• Explains release-local proxy files, early-loading errors, constructor rules, and test preparation.

src/docs/aop.md

broadcasting.mdDocument Pusher coroutine boundaries +4/-0

Document Pusher coroutine boundaries

• Explains same-coroutine asynchronous reuse and alternatives to unsupported connection caps.

src/docs/broadcasting.md

filesystem.mdExplain shared S3 credential providers +2/-0

Explain shared S3 credential providers

• Documents serialization, optional SDK memoization, and tenant-specific credential precautions.

src/docs/filesystem.md

http-client.mdDocument Guzzle ownership and connection settings +24/-3

Document Guzzle ownership and connection settings

• Adds guidance for finishing pending promises in their creating coroutine and explains preserved asynchronous handler settings.

src/docs/http-client.md

mail.mdExplain shared SES credential providers +2/-0

Explain shared SES credential providers

• Documents serialization, memoization boundaries, and pooled-mailer considerations.

src/docs/mail.md

packages.mdClarify repeat class-map registration +1/-1

Clarify repeat class-map registration

• Documents safe same-source registration and rejection of different-source replacements for loaded classes.

src/docs/packages.md

porting-from-laravel.mdAdd migration guidance for Guzzle and Pusher +4/-0

Add migration guidance for Guzzle and Pusher

• Describes pending-work ownership, lazy client registration, and replacing shared-handler connection caps.

src/docs/porting-from-laravel.md

queues.mdExplain shared SQS credential providers +2/-0

Explain shared SQS credential providers

• Documents serialized provider calls and safe SDK memoization.

src/docs/queues.md

testing.mdWarn about coroutine output assertions +3/-0

Warn about coroutine output assertions

• Recommends non-coroutine execution for affected PHPUnit assertions pending upstream support.

src/docs/testing.md

README.mdUpdate HTTP package guidance +2/-0

Update HTTP package guidance

• Documents the package's Guzzle coroutine-ownership integration.

src/http/README.md

Http.phpDescribe new HTTP handler factory +1/-0

Describe new HTTP handler factory

• Adds the newConnectionHandler() facade method annotation.

src/support/src/Facades/Http.php

README.mdDocument benchmark reproduction +51/-0

Document benchmark reproduction

• Lists setup and commands for repeating Guzzle ownership measurements.

tests/Benchmarks/GuzzleOwnership/README.md

results.mdRecord CPU and memory measurements +145/-0

Record CPU and memory measurements

• Reports benchmark comparisons, workload conditions, and capacity implications.

tests/Benchmarks/GuzzleOwnership/results.md

README.mdDocument Guzzle regression coverage +31/-0

Document Guzzle regression coverage

• Describes the ownership test fixtures and validation approach.

tests/Http/Client/Guzzle/README.md

Other (55) +3392 / -200
tests.ymlAdd Guzzle 7 compatibility CI +37/-0

Add Guzzle 7 compatibility CI

• Installs the supported Guzzle 7 and promises 2 dependency family and runs focused ownership and consumer tests.

.github/workflows/tests.yml

.php-cs-fixer.phpDisable unsafe return-assignment formatting +2/-3

Disable unsafe return-assignment formatting

• Prevents rewriting assignments captured by reference in Guzzle wait callbacks.

.php-cs-fixer.php

composer.jsonDeclare broadcasting transport dependencies +3/-0

Declare broadcasting transport dependencies

• Adds direct promises, context, and PSR HTTP-message requirements.

src/broadcasting/composer.json

ClearCommand.phpClear release-local proxy cache +1/-1

Clear release-local proxy cache

• Changes proxy clearing from shared storage to bootstrap/cache/aop.

src/cache/src/Console/ClearCommand.php

composer.jsonRemove unused pipeline dependency +0/-1

Remove unused pipeline dependency

• Drops the dependency after replacing the AOP-specific pipeline.

src/di/composer.json

InteractsWithAop.phpPass instances through AOP test dispatch +2/-1

Pass instances through AOP test dispatch

• Supplies the intercepted object, or null for static calls, to the updated dispatcher.

src/foundation/src/Testing/Concerns/InteractsWithAop.php

composer.jsonDeclare HTTP dependency on DI +1/-0

Declare HTTP dependency on DI

• Adds the AOP package required by Guzzle ownership aspects.

src/http/composer.json

composer.jsonDeclare credential-wrapper dependencies +2/-0

Declare credential-wrapper dependencies

• Adds coroutine and engine requirements used by credential serialization.

src/support/composer.json

testbench.yamlPurge proxies from release-local directory +1/-1

Purge proxies from release-local directory

• Updates the Testbench purge path to bootstrap/cache/aop.

src/testbench/testbench.yaml

composer.jsonDeclare testing promises dependency +1/-0

Declare testing promises dependency

• Adds the Guzzle promises package used by shared PHPUnit queue preparation.

src/testing/composer.json

AfterEachTestExtension.phpPrepare Guzzle proxies before test discovery +58/-0

Prepare Guzzle proxies before test discovery

• Installs a shutdown-disabled test queue and generates framework proxies without enabling optional Sentry or Telescope aspects.

src/testing/src/PHPUnit/AfterEachTestExtension.php

AfterEachTestSubscriber.phpRelease Guzzle state after tests +8/-0

Release Guzzle state after tests

• Cancels outstanding transfers and resets ownership and outside-coroutine queue state during test cleanup.

src/testing/src/PHPUnit/AfterEachTestSubscriber.php

NoopAspect.phpProvide pass-through benchmark aspect +19/-0

Provide pass-through benchmark aspect

• Separates basic dispatch cost from ownership-check cost in measurements.

tests/Benchmarks/GuzzleOwnership/Fixtures/NoopAspect.php

server.phpProvide loopback benchmark server +24/-0

Provide loopback benchmark server

• Serves local responses for outgoing-request benchmarks.

tests/Benchmarks/GuzzleOwnership/Fixtures/server.php

benchmark.phpMeasure ownership overhead across outgoing paths +314/-0

Measure ownership overhead across outgoing paths

• Adds reproducible workloads covering promises, HTTP, SDK consumers, cancellation, startup, and resource use.

tests/Benchmarks/GuzzleOwnership/benchmark.php

PusherClientTest.phpExercise coroutine-safe Pusher clients +361/-0

Exercise coroutine-safe Pusher clients

• Covers concurrent async calls, same-coroutine fan-out, synchronous reuse, custom handlers, and unsupported caps.

tests/Broadcasting/PusherClientTest.php

ClearCommandTest.phpVerify new proxy-clearing location +1/-1

Verify new proxy-clearing location

• Updates the clear-command assertion for bootstrap/cache/aop.

tests/Cache/ClearCommandTest.php

AspectManagerTest.phpTest compiled-chain cache semantics +11/-14

Test compiled-chain cache semantics

• Updates set, get, missing-entry, and flush assertions for closure values.

tests/Di/Aop/AspectManagerTest.php

AspectTest.phpTest constructor targeting +48/-0

Test constructor targeting

• Checks that class rules exclude constructors unless a method rule explicitly selects them.

tests/Di/Aop/AspectTest.php

ProceedingJoinPointTest.phpTest directly supplied instances +10/-39

Test directly supplied instances

• Updates join-point tests for instance and static-call handling without closure reflection.

tests/Di/Aop/ProceedingJoinPointTest.php

ProxyCallVisitorTest.phpTest generated call-site metadata +80/-1

Test generated call-site metadata

• Checks instance forwarding, static null handling, and constructor rule behavior.

tests/Di/Aop/ProxyCallVisitorTest.php

ProxyDispatcherTest.phpTest cached aspect dispatch behavior +204/-18

Test cached aspect dispatch behavior

• Covers priority, short-circuiting, rebinding, nested/concurrent invocations, exceptions, and retained-object safety.

tests/Di/Aop/ProxyDispatcherTest.php

GenerateProxiesTest.phpTest proxy publication and boot validation +270/-20

Test proxy publication and boot validation

• Exercises separate loaders, release-local files, repeat boots, source replacements, regeneration, and missing interception.

tests/Di/Bootstrap/GenerateProxiesTest.php

ClassMapManagerTest.phpTest override loader lifecycle +73/-16

Test override loader lifecycle

• Covers unchanged application maps, same-source re-registration, conflicting loaded sources, and cleanup.

tests/Di/ClassMap/ClassMapManagerTest.php

ServiceProviderDiTest.phpUpdate provider DI registration tests +23/-20

Update provider DI registration tests

• Aligns aspect and class-map assertions with centralized registration and isolated loaders.

tests/Di/ServiceProviderDiTest.php

CoroutineTest.phpAdjust coroutine-list test +3/-1

Adjust coroutine-list test

• Updates coroutine-count assertions for the revised test environment.

tests/Engine/CoroutineTest.php

LoadEnvironmentVariablesTest.phpAdjust environment bootstrap test +3/-0

Adjust environment bootstrap test

• Adds coverage for the revised test setup.

tests/Foundation/Bootstrap/LoadEnvironmentVariablesTest.php

ApplicationBootstrapTest.phpRemove obsolete bootstrap expectation +0/-1

Remove obsolete bootstrap expectation

• Drops an assertion affected by changed proxy preparation or location.

tests/Foundation/Testing/ApplicationBootstrapTest.php

InteractsWithAopTest.phpCheck static AOP instance handling +16/-1

Check static AOP instance handling

• Verifies the test helper supplies null for intercepted static methods.

tests/Foundation/Testing/Concerns/InteractsWithAopTest.php

WithCachedStateTest.phpRemove obsolete cached-state expectation +0/-1

Remove obsolete cached-state expectation

• Drops an assertion affected by changed proxy preparation or location.

tests/Foundation/Testing/WithCachedStateTest.php

BootstrapTest.phpTest ownership activation during bootstrap +91/-0

Test ownership activation during bootstrap

• Covers cold and cached boots, early-loaded targets, and outside-coroutine shutdown behavior.

tests/Http/Client/Guzzle/BootstrapTest.php

CoroutineIsolationTest.phpReproduce cross-request callback isolation +81/-0

Reproduce cross-request callback isolation

• Checks yielding callbacks retain their own request context and database transaction.

tests/Http/Client/Guzzle/CoroutineIsolationTest.php

CoroutineTaskQueueNonCoroutineTest.phpTest outside-coroutine queue compatibility +70/-0

Test outside-coroutine queue compatibility

• Verifies repeated installation preserves the delegate and resets release abandoned test tasks.

tests/Http/Client/Guzzle/CoroutineTaskQueueNonCoroutineTest.php

CoroutineTaskQueueTest.phpTest owner-local callback queues +103/-0

Test owner-local callback queues

• Checks task ordering, child-context isolation, and exit-time draining.

tests/Http/Client/Guzzle/CoroutineTaskQueueTest.php

bootstrap.phpProvide isolated Guzzle boot fixture +95/-0

Provide isolated Guzzle boot fixture

• Sets up proxy and application boot scenarios for subprocess regressions.

tests/Http/Client/Guzzle/Fixtures/bootstrap.php

run-guzzle-promise-tests.phpRun upstream promise compatibility checks +64/-0

Run upstream promise compatibility checks

• Provides a fixture for validating supported Guzzle promise suites against generated proxies.

tests/Http/Client/Guzzle/Fixtures/run-guzzle-promise-tests.php

PromiseOwnershipTest.phpTest pending and adopted promise ownership +142/-0

Test pending and adopted promise ownership

• Verifies foreign pending operations fail while completed results and settled chains remain reusable.

tests/Http/Client/Guzzle/PromiseOwnershipTest.php

TransportOwnershipTest.phpTest transport admission and cleanup +410/-0

Test transport admission and cleanup

• Covers yielding preparation, foreign driving, owner exit, cancellation propagation, retry callbacks, and handler reuse.

tests/Http/Client/Guzzle/TransportOwnershipTest.php

HttpConnectionTest.phpVerify asynchronous connection settings +53/-11

Verify asynchronous connection settings

• Checks isolated handlers preserve transport sharing, multiplexing, and registered settings after preset overrides.

tests/Http/HttpConnectionTest.php

AboutCommandTest.phpCheck release-local proxy reporting +2/-2

Check release-local proxy reporting

• Updates about-command expectations for bootstrap/cache/aop.

tests/Integration/Foundation/Console/AboutCommandTest.php

DatabaseStoreTest.phpFreeze timing-sensitive rate-limit tests +12/-5

Freeze timing-sensitive rate-limit tests

• Stabilizes contract decisions against I/O scheduling delays.

tests/Integration/RateLimiter/Database/Sqlite/DatabaseStoreTest.php

QueueSqsConnectorTest.phpVerify wrapped callable SQS credentials +6/-4

Verify wrapped callable SQS credentials

• Updates assertions to distinguish callable providers from unchanged credential objects.

tests/Queue/QueueSqsConnectorTest.php

SwooleStoreTest.phpStabilize rate-limit clock behavior +5/-0

Stabilize rate-limit clock behavior

• Adds clock control for timing-sensitive store assertions.

tests/RateLimiter/SwooleStoreTest.php

WorkerArrayStoreTest.phpStabilize worker-array rate-limit tests +6/-0

Stabilize worker-array rate-limit tests

• Adds clock control for timing-sensitive decisions.

tests/RateLimiter/WorkerArrayStoreTest.php

ResponseFactoryTest.phpAdapt streaming output tests +4/-3

Adapt streaming output tests

• Adjusts response-stream assertions for coroutine output-capture limitations.

tests/Routing/ResponseFactoryTest.php

AuthenticatesRequestsTest.phpAdapt synchronous output assertions +14/-21

Adapt synchronous output assertions

• Runs affected authentication output checks outside the test coroutine.

tests/Saloon/Feature/AuthenticatesRequestsTest.php

AwsCredentialConsumerTest.phpTest shared credentials across pooled SDK clients +129/-0

Test shared credentials across pooled SDK clients

• Checks caller-specific signing and serialization for SQS, S3, and SES.

tests/Support/AwsCredentialConsumerTest.php

SerializedCredentialProviderNonCoroutineTest.phpCheck provider outside coroutines +34/-0

Check provider outside coroutines

• Verifies provider work completes under non-coroutine execution.

tests/Support/SerializedCredentialProviderNonCoroutineTest.php

SerializedCredentialProviderTest.phpTest credential serialization and recovery +330/-0

Test credential serialization and recovery

• Exercises callable aliases, memoization, caller-specific results, failures, and canceled owners or waiters.

tests/Support/SerializedCredentialProviderTest.php

SupportComposerTest.phpCheck application-loader discovery +17/-0

Check application-loader discovery

• Ensures an auxiliary prepended loader does not replace the discovered Composer application loader.

tests/Support/SupportComposerTest.php

PurgeSkeletonCommandTest.phpCheck new Testbench purge target +2/-2

Check new Testbench purge target

• Updates proxy-directory cleanup expectations.

tests/Testbench/Foundation/Console/PurgeSkeletonCommandTest.php

AfterEachTestExtensionTest.phpTest early proxy setup and queue cleanup +20/-0

Test early proxy setup and queue cleanup

• Checks proxies are ready before discovery without optional instrumentation and callbacks do not leak between tests.

tests/Testing/PHPUnit/AfterEachTestExtensionTest.php

GuzzleCleanupFixture.phpProvide pending-callback cleanup fixture +36/-0

Provide pending-callback cleanup fixture

• Leaves an outside-coroutine task pending and checks it is released before the next test.

tests/Testing/PHPUnit/Fixtures/GuzzleCleanupFixture.php

TimeLimitFixture.phpProvide yielding timeout scenarios +37/-0

Provide yielding timeout scenarios

• Adds sleeping, child-coroutine, completed-work, and non-yielding timeout fixtures.

tests/Testing/PHPUnit/Fixtures/TimeLimitFixture.php

TimeLimitTest.phpTest coroutine timeout reporting +53/-12

Test coroutine timeout reporting

• Checks suspended and non-yielding tests fail under their test identity while completed work does not.

tests/Testing/PHPUnit/TimeLimitTest.php

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. A second autoloader hides app proxies 🐞 Bug ≡ Correctness
Description
Composer::findLoader() selects the first registered Composer loader without checking whether it
owns the application classes. If a loader for another vendor directory was registered earlier and
the application loader was subsequently prepended, proxy generation reads the other loader’s class
map and cannot find application targets.
Code

src/support/src/Composer.php[R284-287]

+        $loaders = ClassLoader::getRegisteredLoaders();
-        foreach ($loaders as $loader) {
-            if (is_array($loader) && $loader[0] instanceof ClassLoader) {
-                return $loader[0];
-            }
+        if ($loaders !== []) {
+            return reset($loaders);
Evidence
The changed selection takes the first entry unconditionally. Proxy generation then uses that
loader's class map and findFile() results to determine which classes can receive proxies; the
existing auxiliary-loader test does not cover an earlier-registered vendor-directory loader.

src/support/src/Composer.php[279-290]
src/di/src/Bootstrap/GenerateProxies.php[150-174]
tests/Support/SupportComposerTest.php[33-51]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Composer::findLoader()` assumes the first registered Composer loader belongs to the application. With multiple vendor-directory loaders, proxy generation can use a different loader and miss application classes.
## Fix Focus Areas
- src/support/src/Composer.php[279-290]
- src/di/src/Bootstrap/GenerateProxies.php[150-174]
- tests/Support/SupportComposerTest.php[33-51]
## Recommended Fix
Select the loader that owns the application's autoload paths rather than the first registered loader. Add a regression test with an earlier-registered vendor-directory loader and a subsequently prepended application loader.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Concurrent AWS SDK calls can hang their coroutine forever ✓ Resolved
Description
SerializedCredentialProvider::__invoke takes the non-reentrant Locker lock and then blocks on
($this->provider)(...)->wait(). When the inner promise is pending, Guzzle's wait() drains the
current coroutine's task queue while the lock is still held. That is the normal case for a memoized
expiring provider, because memoize returns $result->then(...), and it is also the case during a
refresh. If a queued task starts another SDK command on a client using the same provider, it calls
Locker::lock on the same key. A typical case is the next part of an S3 multipart upload, a
CommandPool refill, or another EachPromise step. Locker::lock then pops a channel that only
this same suspended frame can close, so the coroutine never resumes.
Code

src/support/src/Aws/SerializedCredentialProvider.php[R55-60]

+        while (! Locker::lock($this->lockKey)) {
+            // The previous caller finished; compete to run the provider next.
+        }
+
+        try {
+            return Create::promiseFor(($this->provider)(...$arguments)->wait());
Evidence
Locker::lock only returns true the first time for a key. Any later call, including one from the
coroutine that already holds the lock, waits with $channel->pop($timeout) (timeout=-1 by
default) until unlock() closes the channel. In the wrapper, unlock() runs only in the finally
of the outer frame that is blocked in wait(). Guzzle's Promise::wait() on a pending promise runs
Utils::queue()->run(). With this PR, that is the per-coroutine queue (CoroutineTaskQueue::run ->
CoroutineState::forCurrent()->queue), so earlier-queued callbacks from the same coroutine run
inside the locked section. Flysystem S3 uploads (MultipartUploader, concurrency 3), CommandPool,
and other async SDK fan-out start new commands from those callbacks. Each new command signs its
request by calling the credential provider. With the user-configured callable providers that
FilesystemManager::createS3Client, MailManager::createSesV2Transport and
SqsConnector::resolveCredentialProvider now wrap, the lock is re-entered and the coroutine blocks
forever.

src/support/src/Aws/SerializedCredentialProvider.php[53-73]
src/coroutine/src/Locker.php[31-51]
src/http/src/Client/Guzzle/CoroutineTaskQueue.php[57-82]
src/filesystem/src/FilesystemManager.php[562-569]
src/queue/src/Connectors/SqsConnector.php[130-139]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SerializedCredentialProvider::__invoke` holds a non-reentrant `Locker` lock while calling `->wait()` on the provider promise. When that promise is pending, Guzzle's `wait()` drains the current coroutine's task queue. A queued callback that starts another SDK command signs it by calling the same wrapped provider again. That nested call reaches `Locker::lock` for a key the coroutine already holds and waits forever.
## Fix Focus Areas
- src/support/src/Aws/SerializedCredentialProvider.php[53-73]
## Recommended Fix
Record which coroutine owns each lock key, for example in a static `array<string, int>` keyed by `$this->lockKey` and holding `Coroutine::id()`, plus a depth counter. If the current coroutine already owns the key, skip `Locker::lock`/`unlock` and invoke the provider directly. A nested call is in the same coroutine, so it already meets the ownership rule. Otherwise acquire the lock as today, record ownership, and clear it in `finally` before `Locker::unlock`. Add a regression test in which a queued task, drained during the provider's pending `wait()`, invokes the same wrapper again in the same coroutine and must complete instead of hanging.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/support/src/Composer.php
Comment thread src/support/src/Aws/SerializedCredentialProvider.php Outdated
Waiting for memoized credentials drains Guzzle callbacks. A callback starting another SDK request could wait for a provider lock held by the same coroutine and deadlock.

Keep cross-coroutine serialization while allowing nested calls by the native owner. Only the outermost frame acquires and releases the lock, and every invocation still waits for its own completed result. Clear the bounded owner map during normal release and test cleanup.

Cover nested requests through two framework-created S3 clients using callable aliases, cached credentials, refreshes and subsequent use from another coroutine. The regression hangs with reentry disabled. Full framework, Testbench, package integration, formatting and static analysis checks pass. Clarify that existing credential benchmarks predate this correction.
The early-loaded target test correctly triggered its bootstrap exception in CI, but PHP wrote it to standard output while the assertion read standard error. Set the subprocess error destination explicitly.

Remove obsolete manual-interception fallbacks from Sentry and Telescope tests now that the PHPUnit extension generates their Guzzle proxies before discovery. Correct the testing helper guidance without weakening bootstrap validation.

Bootstrap, Sentry and Telescope tests pass, along with formatting and static analysis. The full framework and package-mode suites also pass.
Use SplObjectStorage::offsetExists() for both shared-reference lookups instead of contains(), which is deprecated in PHP 8.5. The replacement is available on PHP 8.4 and preserves object membership semantics.

Verified on PHP 8.4, including stored null values. Notification transport serialization tests and the full framework suite pass.
@binaryfire
binaryfire merged commit 3009105 into 0.4 Oct 9, 2026
52 checks passed
@binaryfire
binaryfire deleted the fix/guzzle-coroutine-ownership branch October 10, 2026 17:14
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