[stevehu]

Fixes #2820. Target: master. Draft for shared-code review and performance qualification.

ExternalServiceHandler's asynchronous work can escape the chain's configured ExceptionHandler and instantiate a default sibling policy. Carry the selected named/enabled policy, including its deliberate absence, across worker dispatch, suspended body reads, HMAC completions and proxy callbacks. Preserve established typed-error bodies, raw JSON logs and ERR10011 / HTTP 400 mapping.

Scope captures lexical policy before hand-off. A fresh capture outside a frame has no policy; custom asynchronous handlers must retain their captured scope. Earlier sub-pipeline boundaries cannot shadow later boundaries. Framework MDC uses immutable generations in a stable holder, published during serial bootstrap before asynchronous hand-off. RequestState's concurrent weak-identity index contains only MDC, never policy closures. Initialization no longer registers an exchange-completion listener. Collected exchanges are removed by bounded ReferenceQueue expunging on later registrations; idle stale entries can remain until another registration. Captured resumes read their holder directly.

The dispatcher is now package-private in proxy-handler, with two atomic fields. Its FIFO handles both owner and foreign arrivals. Fatal owner cleanup runs on the I/O owner before the terminal state is published. Executor rejection notifies exactly once on the rejecting submission, closes a separately published immutable upstream-resource lease, and reports the failure through the selected scope. This fallback reads no non-volatile proxy request/connection/attempt/stopping fields and never mutates them. Later stopped submissions close only their own incoming resources; they never invoke owner cleanup or repeat the policy. Closing a multiplexed request targets its channels, preserving the HTTP/2 parent. Rejection handling is best effort when the owning I/O executor is unavailable.

The public HandlerContinuation.Dispatcher, both Scope.isActive overloads, Scope.fail and the public generic dispatcher seam are removed. Scope keeps only operations used by production callers. ExceptionHandler no longer implements HandlerWrapper or exposes a public wrap method. It supplies its private policy through the native Undertow wrapper at the existing entry point.

RequestInterceptorInjectionHandler captures both its final downstream wrapper and a cached listener task when the body first suspends. The listener itself now uses that captured policy, including failures outside resumeRead's body-read catch. Pending reads remain pending. Nested MDC frames reuse the active context.

Ordinary and fatal fallback cleanup retains the primary failure, adds failed writes/resets as suppressed exceptions, and logs those failures. Diagnostic failures are guarded so they cannot replace the primary failure. The identity and suppression assertions are restored. No rule-keyed Sonar suppressions are added; explicit Error/Throwable guards remain visible.

The existing attempt guards, public CONNECTION attachment, HTTP/2 child-exchange push handling, independent child MDC, retry-state reset and stream-specific cancellation remain. Proxy-owned terminal responses leave benign late callbacks draining.

Validation on each branch:

• Clean focused reactor run: 177 continuation/proxy/body/HMAC/MCP tests pass.
• A further 120-test handler/exception/listener/proxy/SSE repeat passes.
• After integrating current master / 2.x server-option changes unchanged, a clean expanded run passes 198 tests on each branch, including ServerConfigTest and ServerOptionRegistryTest.
• Final handler/listener cleanup passes another 24 tests per branch.
• New proxy-local tests exercise deterministic foreign-before-owner ordering, 32,000 callbacks from four concurrent producers across 40 drain/idle cycles, exactly-once executor-rejection notification, queued fatal failure, owner-thread abort, 100 subsequent foreign submissions, isolated resource retirement and suppressed cleanup failures.
• New listener test injects a failure outside resumeRead's try/catch and verifies selected policy handles the original failure. Restoring the old off-frame task makes this test fail with no selected failure observed.
• Weak-index test verifies both an abandoned exchange and its state are collected after expunging. Removing INDEX.remove makes the test fail: the exchange is collected while its state remains retained. Code is restored and positive runs pass.
• Completion-race fixture makes listener registration throw IllegalStateException; initialization never calls it, both before and after completion.
• Existing real HTTP/2 push, sibling-stream survival, fatal HTTP/1.1/HTTP/2 lifecycle, retries/late completions, disabled policy, MDC reader/writer, typed JSON/log synchronization, interruption and SSE cases pass.

Expanded command:

mvn -pl handler,exception,ingress-proxy,hmac,dump,mcp-router -am clean test -Dtest=ExternalServiceWorkerDispatchTest,ExternalServiceInterruptTest,HandlerContinuationTest,RequestInjectionContinuationTest,ExceptionContinuationTest,HandlerTest,HmacPhase0IntegrationTest,ProxyContinuationTest,ProxyCallbackDispatcherTest,BodyContinuationTest,DumpContinuationTest,McpContinuationTest,ProxyHandlerSseTest,HmacHandlerIntegrationTest,HmacReplayAdminHandlerTest,ServerConfigTest,ServerOptionRegistryTest -Dsurefire.failIfNoSpecifiedTests=false

Allocation evidence is outside the repository; no standalone probe source or machine-specific JSON is committed. A Corretto 25 ThreadMXBean component probe now includes exchange creation, initial scope/frame, immutable MDC publication, weak-index registration, dispatcher/queue creation, four capturing callback registrations and wrapper execution (20k warm-up; three 20k samples). Across review-11 runs, component allocation was approximately 1.25-1.31 KB/request with empty MDC and 2.97-3.40 KB with three fields. Final merged-code samples: master 1251-1310 / 3394-3396 bytes (empty / three fields), 2.x 1251-1304 / 2978-2980 bytes. Warm-up/JIT differences are visible; these figures are not a comparative performance guarantee. The component probe excludes actual transport and the private shutdown lease.

A separate real HTTP/1.1 loopback runs ExceptionHandler + ProxyHandler + an upstream server: 300 warm-up requests and three 500-request samples per case. All-Java-thread allocation totals, including the Java HTTP client and both transports, were approximately 19 KB/request for direct upstream requests, 129-131 KB for the proxied empty-MDC case and 131-133 KB with three MDC fields. Samples reported no thread creation/loss. These are total loopback figures, not allocation attributable solely to the continuation patch, and not production throughput qualification.

The previous master head's full source-build and native-binding CI passed. Fresh final-head checks are tracked separately; the 2.x branch has no source-build Actions workflow. Local complete-suite success is not claimed because legacy fixtures bind occupied port 7080. Shared-code review and gateway performance qualification remain open. Worker saturation/backpressure, Salesforce/MRAS qualification and changing ERR10011 to 502/504 remain outside scope.

Remote evidence on the merged code heads (7ce8033 master / 96fe84f 2.x): SonarCloud quality gate passes, with zero bugs, zero vulnerabilities and eight visible S1181 findings for intentional fatal-error and diagnostic guards. Pre-commit passes. Master source/native jobs are running. A documentation-only follow-up widens the allocation range to include observed JIT variation; production Java source and bytecode hashes are recorded in the local qualification evidence.

[stevehu]

Review findings, most significant first. I read the diff and surrounding code only and did not run the tests.

1. Boundary is only restored at two opt-in sites (HandlerContinuation.capture). After a streamed body resumes on the I/O thread, downstream handlers that re-dispatch themselves lose the exception policy: BodyHandler:108, DumpHandler:80, McpHandler:176, ProxyHandler:211/407, HmacHandler:133 and HmacReplayAdminHandler:106. HmacHandler even logs and rethrows expecting ExceptionHandler to catch it. The client then gets Undertow's bare 500 instead of ERR10011. Consider one worker hop in RequestInterceptorInjectionHandler.handleEvent using the captured continuation, or a shared HandlerContinuation.dispatch(...) helper.
2. No active boundary now means a bare 500 instead of 400/ERR10011. Previously ExternalServiceHandler always dispatched inside a default ExceptionHandler. With exception handling disabled or omitted, an upstream IOException now returns an empty-body 500 plus an UT005071 log. The README documents this, but it is a client-visible change that is worth confirming.
3. ExceptionHandler.handleContinuation now runs once per resumed frame. The comment about avoiding double logs is stale, and MDC.clear() runs on whichever thread resumes, including the I/O thread.
4. Eager capture in the RequestBodyReadState constructor. It allocates on every body-reading request, though it is only used on the async path. The field and parameter are both named continuation. Capture lazily in awaitMoreData().
5. Test: withPolicy clobbers shared config. The concurrent test calls it four times against the same cache keys, so it passes only because isEnabled() is sampled at build time. The teardown putInConfigCache(..., previous) also NPEs if the previous value is null.
6. Test: assertRawFailure does response.split(" ", 3)[1] first, so an empty response gives an opaque ArrayIndexOutOfBounds. Assert the status line first.
7. No tests for the integration in the owning modules. exception and handler have no test for the ExceptionHandler boundary or the read-listener resume. Only the ingress-proxy socket tests cover them. Please run the exception, handler, server and body suites once CI is green, since local runs were limited by the port 7080 conflict.
8. Simplification. HandlerContinuation.execute has three null branches and stores a null attachment value to mean "deliberately absent". A no-op sentinel boundary would be simpler. The Boundary Javadoc should say that implementors must call the continuation exactly once.

[stevehu]

Re-review of the follow-up commit. I read the diff and surrounding code only and did not run the tests. The earlier items are addressed, except the test-teardown NPE in item 7. The commits are identical on master and 2.x apart from the Jackson import, so this applies to both PRs.

1. MDC cost on the ProxyHandler hot path (HandlerContinuation.capture, line 68). Each proxied request now does two captures (ProxyClientHandler and ProxyAction) and two resumed frames. That is roughly 8 HashMap copies plus several lambda, UnaryOperator and Runnable allocations, whether or not any MDC is set. ProxyAction could reuse the ProxyClientHandler continuation. The snapshot could be skipped when the MDC is empty. Please benchmark the proxy path, since GitNexus already rates this scope as critical.
2. ProxyContinuationTest passes through an incidental NPE (line 24). new ProxyConnection(null, "/") makes ProxyAction.run() throw at its first getConnection().getPeerAddress(), and that NPE is the failure the test asserts on. Nothing tests the new contract that a successful callback leaves the exchange pending. Please add a case with a real or stub upstream connection.
3. Retry, timeout and response callbacks are still unwrapped. failed() calls findTarget and getConnection (line 433) directly, and cancel() and the ClientCallback/ResponseCallback paths also run with no boundary. These could reuse the continuation already held by ProxyClientHandler and ProxyAction.
4. task() ends the exchange on any Throwable, even after upstream I/O has started. If ProxyAction.run() throws after sendRequest, executeRootHandler calls endExchange() while the callbacks are still in flight. Consider ending only when no response has started and nothing was dispatched upstream.
5. MDC propagation now exists twice. MiddlewareHandler.addHandlerMDCContext stores it in the MDC_CONTEXT attachment, which LightThreadExecutor re-applies. HandlerContinuation then overwrites that with a thread-local snapshot. The two can disagree. Please keep one mechanism.
6. Cleanup. DumpHandler:80 and McpHandler:176 use an inline fully qualified HandlerContinuation, unlike BodyHandler. The same boundary and server scaffolding is copy-pasted across six continuation tests, so a shared test fixture would help.
7. Test teardown NPE. ExceptionContinuationTest (finally block) and ExternalServiceWorkerDispatchTest call putInConfigCache(name, previous), which throws if previous is null. Use clearConfigCache in that case.

[stevehu]

Third-round review of the latest commit (03ef782 on master, 441ec6f on 2.x). I read the diff and surrounding code only and did not run the tests. The earlier items are addressed: the shared LightThreadExecutor field race is gone, the per-request MDC copies are removed, retry/timeout/send/response callbacks are covered, the proxy tests now use a real upstream, and the null-config teardown is fixed. The two commits are identical apart from README line numbers, so this applies to both PRs.

1. ExceptionHandler now closes the connection after the response has started (ExceptionHandler.java:104). This affects every service, not only the proxy. isResponseStarted() is true once the first bytes are committed, but isComplete() stays false until the body is flushed. A handler that sends a large response (async write) and then throws used to be only logged. Now the connection is closed, which can truncate the response, and on HTTP/2 it kills unrelated streams. The new branch has no test in the exception module. Consider limiting this to the proxy's task() path.
2. LightThreadExecutor now runs on every proxy callback (ProxyHandler.java:396). That is about five or more calls per request, including ResponseCallback on shared client I/O threads and the timeout timer. It clears the MDC without restoring it. It also logs ERROR whenever MDC_CONTEXT is absent, which is the case when correlation or traceability is disabled. MDC_CONTEXT is a plain HashMap that can be iterated here while a worker mutates it. Downgrade the log and save/restore the previous MDC.
3. Nested wrapping (ProxyHandler.java:449). handleFailed calls the public wrapped cancel and couldNotResolveBackend. completed() also wraps the ProxyAction twice, with a no-op INLINE_EXECUTOR dispatch in between. Call the private unresolvedBackend and cancelRequest directly and use one wrapper in completed().
4. Shared connections are closed on failure (ProxyHandler.java:391). continuationTask closes the upstream ClientConnection and task() closes the downstream connection. On multiplexed HTTP/2 connections that kills unrelated in-flight streams.
5. task() swallows Error (HandlerContinuation.java:85). OutOfMemoryError and StackOverflowError are now logged and turned into a 500 or a close, instead of being rethrown into Undertow's path.
6. Resumed frames lose the request MDC and never clear it (ExceptionHandler.java:84). Only the initial frame clears. Frames resumed through capture() (ExternalServiceHandler worker, Body/Dump/Mcp, RequestInterceptor resume, HMAC) have no correlationId in their logs. A direct MDC.put downstream, for example CorrelationHandler:65/87, leaks onto the thread.
7. Missing tests (ExceptionContinuationTest.java:45). Nothing covers the new isComplete/isResponseStarted branches, the connection close, the missing-MDC_CONTEXT path, or HTTP/2.
8. Hygiene (ProxyHandler.java:598). The ProxyAction constructor's closing brace is misindented and imports are out of order. ExceptionHandler uses org.xnio.IoUtils without the exception module declaring an xnio dependency.

[stevehu]

Fourth-round review of the latest commit (45cba7f on master, 7b9e022 on 2.x). I read the diff and the Undertow sources only and did not run the tests. The general ExceptionHandler close is gone, and Http2ServerConnection.close() does send RST_STREAM for that stream only. Nested proxy frames are gone, MDC is scoped per frame from MDC_CONTEXT, and HTTP/2 cancellation is stream-scoped. The two commits are identical apart from README line numbers, so this applies to both PRs.

1. Error leaves the downstream request hanging (HandlerContinuation.java:111). task() now rethrows Error, but only catch (Exception) calls failTask. A StackOverflowError or OOM in a proxy callback cleans up the upstream via abortUpstream, but the downstream exchange gets no 500, endExchange or stream reset. The client waits for its own timeout and the front-end connection stays pinned. The same applies to RequestInterceptorInjectionHandler.handleEvent. The test only asserts the Error is rethrown and the MDC restored. Please call failTask best-effort in a finally and then rethrow.
2. ABORTING is only honored in boundaryCallback (ProxyHandler.java:481). ProxyClientHandler.completed(), failed() and queuedRequestFailed() ignore it. After a timeout cancel writes 504 and the request body is still draining, isComplete() is briefly false, so a late completed() can start a ProxyAction on an already answered exchange. Also, CLIENT_EXCHANGE is never cleared on success, so a late abort could close an HTTP/1.1 connection that was already released to the pool and reused.
3. Two thread-local reentrancy guards (ProxyHandler.java:391). ACTIVE_CALLBACK and HandlerContinuation.MDC_EXCHANGE answer the same question with different keys. The nested-reuse branch skips abort and failure handling and relies on an outer frame. HandlerContinuation.task() also has unreachable catch (RuntimeException) and catch (Exception impossible) clauses.
4. Per-frame MDC copy work (HandlerContinuation.java:87). withMdc does getCopyOfContextMap plus setContextMap on four or more frames per proxied request. MDC_CONTEXT is an unsynchronized HashMap that setContextMap iterates from I/O threads while a worker could still be adding to it. Skip the swap when nothing changes, and snapshot the map immutably.
5. Vacuous test assertion (ProxyContinuationTest.java:53). It asserts the LightThreadExecutor logger is silent, but ProxyHandler no longer uses that class, so it can never fail. The failure message is stale. LightThreadExecutor is now dead code in main but public.
6. Import churn (ProxyHandler.java:20). The whole import block is reordered in a file flagged HIGH, which inflates the diff and risks conflicts with the Ensure Http2Client sends use the owning Undertow I/O thread #2817 branches.
7. HTTP/2 test coverage (ProxyContinuationTest.java:234). The new sibling-stream tests cover only the response-callback failure. HTTP/2 timeout cancellation, SEND_CALLBACK and ACQUIRE on a shared connection, and late completed() after an abort are untested.

[stevehu]

Fifth-round review of the latest commit (3efe64e on master, b814e36 on 2.x). I read the diff only and did not run the tests. The merge of master brings in #2834 but does not interact with this code, because ProxyHandler uses only Undertow's own client types. The import churn is gone. The fatal-error cleanup, the ABORTING and response-started guards, the single frame tracker and the immutable MDC snapshots address the earlier items. The two commits are identical apart from README line numbers, so this applies to both PRs.

1. synchronized (exchange) does not protect the attachment map (MiddlewareHandler.java:71, also HandlerContinuation.java:108). MDC_CONTEXT lives in Undertow's unsynchronized IdentityHashMap, and callback threads still write other keys such as CLIENT_EXCHANGE and ABORTING without the lock. The lock therefore removes no structural race and adds a monitor acquisition per frame on a public object. The attachment also becomes an unmodifiable map on a public key (AttachmentConstants.MDC_CONTEXT), so any external handler that calls put on it now throws UnsupportedOperationException. Publishing the snapshot is enough. Otherwise use a private lock, and document the key as read-only.
2. A fatal Error before commitment resets the connection instead of returning a 500 (HandlerContinuation.java:132). Every Exception path returns 500, but a recoverable StackOverflowError now closes the whole connection. On HTTP/1.1 that drops keep-alive and pipelined requests. With ExceptionHandler installed the same error returns ERR10011, so behavior depends on the chain. Try the 500 or stream reset first, close only if the response has started or that fails, then rethrow.
3. The new guards return silently (ProxyHandler.java:489, also :472). completed() and handleFailed() return when isResponseStarted() is true but the exchange is not complete, and nothing ends, closes or logs it. The ABORTING case is fine because the abort path already ended the exchange. Please log the dropped callback and close the downstream stream in the started-but-incomplete case.
4. Per-frame allocations remain (HandlerContinuation.java:100). Each resumed frame allocates two Frame records and copies the thread MDC twice, and it installs the request map even when it is unchanged. With the immutable snapshot you could skip the swap on identity, and use one frame object. This is a good first place to measure for the allocation profiling.
5. Test gaps (HandlerContinuationTest.java:36). The MDC publication test uses concurrent writers only, never a reader racing a writer, so it cannot fail if the immutability is removed. Error with ExceptionHandler installed, and on an HTTP/1.1 downstream, are untested.
6. Package-private access across modules (HandlerContinuation.java:47). isActive is used by ProxyHandler through a split com.networknt.handler package across two jars. That breaks under JPMS, sealed jars or separate classloaders. Please make it a public, documented entry point.

[stevehu]

Review of the current heads (3efe64e on master, b814e36 on 2.x) with the SonarCloud result. These are the same commits as the previous round, so the code findings carry over. I read the diff and SonarCloud's public API only and did not run the tests. The two PRs are identical apart from README line numbers.

The failing "SonarCloud Code Analysis" check comes from one rule. java:S2445 is flagged at MiddlewareHandler.java:71 and HandlerContinuation.java:108 (synchronized (exchange) on a method parameter). Those are the only two BUG-type issues on PR 2836, and they drive new_reliability_rating to C (threshold A). The other gate conditions pass. This is the same synchronized (exchange) concern from the previous round: the lock also does not protect the exchange's unsynchronized attachment map, and the attachment is now an unmodifiable map on a public key. The immutable snapshot published with putAttachment is enough on its own. Otherwise use a private lock or an AtomicReference.

Still open from the previous round:

1. A fatal Error before commitment resets the connection instead of returning a 500 (HandlerContinuation.java:132). Sonar also flags the broad catch (Error) and catch (Throwable) (S1181). Try the 500 or stream-reset cleanup first, close only if the response has started or that fails, then rethrow.
2. Late-callback guards return silently (ProxyHandler.java:489). completed() and handleFailed() do nothing when the response has started but the exchange is not complete, so the exchange can hang. Please log the dropped callback and close the downstream stream.
3. Per-frame allocations remain (HandlerContinuation.java:100). Two Frame records and MDC copies per resumed frame, even when the snapshot is unchanged. This is the open allocation-profiling gate. Compare the snapshot by identity and skip the swap.
4. isActive is package-private across two modules (HandlerContinuation.java:47). It relies on a split com.networknt.handler package, which breaks under JPMS, sealed jars or separate classloaders.
5. Test gap (HandlerContinuationTest.java:36). The MDC publication test uses concurrent writers only, never a reader racing a writer. Error with ExceptionHandler installed, and on an HTTP/1.1 downstream, are untested.

New this round, from Sonar, not gate-failing:

6. Cognitive complexity and test style. java:S3776 on HandlerContinuation.task (20 > 15) and ProxyHandler.handleFailed (16 > 15), empty test methods (S1186) at HandlerContinuationTest:44 and ProxyContinuationTest:415/416, and about ten S7467 unused-variable hits. Cleaning these up together with the S2445 fix would stop them recurring on every push.

[stevehu]

Code review — round 7 (heads 79301b4 / e59abff)

The two PRs carry the same change apart from README line numbers. 14 findings, most severe first.

Correctness

1. CI test is red on Preserve chain exception policy across asynchronous continuations #2836. ProxyContinuationTest.asynchronousFailuresUseSelectedBoundaryAndCloseUpstream[8] (RETRY, boundary on, MDC on) fails with "expected <1> but was <2>" (run 41085160884). The test's depth counter is global across threads. In RETRY the JUnit thread runs callback.failed as soon as retry.complete is called inside the I/O-thread frame, so two frames overlap. The test counts thread overlap, not nesting. It passes locally, so it is timing-dependent. [2.x] Preserve chain exception policy across asynchronous continuations #2837 shows no test job, and I did not check why.
2. Attachment map is still raced. The holder removes writes to the MDC key, but execute still puts and removes ACTIVE, and ProxyHandler still writes CLIENT_EXCHANGE, ABORTING and CONNECTION, from overlapping frames. A concurrent getAttachment(MDC_CONTEXT) can briefly miss, so initialize could install a second empty holder over the real one and drop the request MDC. One per-exchange state object created before the first hand-off would fix this at the root.
3. RequestMdcContext.read throws IllegalStateException for a non-holder map, before the callback runs and outside invokeTask's catch. A logging mismatch can leave the exchange without a response. Log and degrade instead.
4. failTask is not hardened like failFatal. A throwing endExchange(), setStatusCode or non-IO close() replaces the original failure.
5. terminalCallback has side effects behind a predicate name. It resets the client connection for any late callback after the response has started, including one that arrives while a proxy-generated 503/504 is still draining the request body.

Tests
6. The reader/writer test uses only a pre-initialized holder through package-private snapshot()/putFields. Overlap is not guaranteed, and the production read path, the initialization race and legacy-map migration are untested.
7. The late-callback termination tests are HTTP/1.1 only.

Cleanup
8. Four helpers (failTask, failFatal, closeAfterFatal, rememberCleanupFailure) implement the same state machine with different guards.
9. putFields is dead in production. RequestMdcContext re-reads current on every call and has an O(n) containsKey.
10. withMdc still makes two full MDC copies per frame, and no allocation measurement came with the patch.
11. task() special-cases CapturedHandler with a duplicate Runnable.
12. The early RequestMdcContext.initialize in ExceptionHandler before the dispatch is redundant, and a test pins it.
13. MiddlewareHandler has an unused import (S1128). Sonar reports 24 smells on #2836 and 23 on #2837. New in this patch: S1181 ×4 in HandlerContinuation, S2160, S6878 and S5778.
14. The new tests add many inline fully qualified names instead of imports.

SonarCloud passes on both PRs (0 bugs). Both PRs remain drafts.

[stevehu]

Code review — round 8 (heads 4df4bf6 / 139bcf8)

The two branches carry the same change. The only difference in the new commits is the machine-specific numbers in allocation.json. CI on #2836 is green (test, native-binding, pre-commit) and SonarCloud has no open issues on either PR. Both PRs remain drafts.

This patch resolves most earlier findings:

• The ACTIVE attachment is gone and the boundary is thread-local.
• The throwing read() is gone.
• The termination helpers are merged.
• Late callbacks after the proxy's own 503 are now ignored.
• The depth counter in the RETRY test is thread-local.

9 findings remain, most severe first.

Correctness

1. Server push runs on the wrong exchange. ProxyContinuation.run(exchange, callback) ignores exchange, because resumed is bound to the constructor's exchange (ProxyHandler.java:1090). The push path builds its ProxyAction against the original request, so pushed responses are copied onto it. No test covers push.
2. Monitor held across whole callbacks. run is synchronized for the entire callback, including ProxyAction.run, Transfer.initiateTransfer, endExchange() and the user's boundary. It also passes the callback through a mutable shared callback field. A Boundary that calls its continuation asynchronously sees null.
3. Public ProxyHandler.CONNECTION is never set. The connection moved to a private field, so external readers silently get null.
4. Stale retry state. request and connection are not reset between attempts. A timeout during a retry can close the old attempt's connection and leave the new one open.
5. capture() is thread-scoped. Called off-frame it silently returns UNHANDLED, and the class contract was rewritten to match. Worth documenting and logging.

Cleanup
6. ExceptionHandler reads a global static twice. ClientException.getStatus() is a JVM-global static, read twice, so concurrent requests can see each other's status. The refactor is also out of #2820's scope and changes the typed-failure log message.
7. Circular coupling. RequestMdcContext.initialize and HandlerContinuation.activeContext depend on each other through a thread-local, so the public helper's result depends on hidden thread state.
8. Benchmark files committed. handler/qualification/ holds an uncompiled probe and machine-specific JSON that CI never builds. The two branches already record different numbers.

Tests
9. Nothing covers push, the monitor, retry-state reset, off-frame capture or the CONNECTION key.

[stevehu]

Code review — round 9 (heads 2b2788d / fe1ea0f)

The two branches carry the same change apart from the numbers in the deleted benchmark files.

CI. test is red on #2836. ExceptionContinuationTest.typedFailuresRetainResponseStatusAndBody[3] (api, not resumed) fails on the new log assertion (run 41116029655). handleContinuation sends the response first and logs afterwards, and the test reads the appender's plain ArrayList as soon as the client has the response. #2837 shows only SonarCloud and pre-commit checks, with no test job. SonarCloud passes on both PRs with no open issues. Both PRs remain drafts.

Fixed since round 8:

• Server push now runs on its own exchange.
• The callback-wide monitor is gone.
• The public CONNECTION attachment is populated again.
• Retries release the previous attempt's connection before the next one is acquired.
• ExceptionHandler reads the client status once.
• The benchmark files are removed.

10 findings remain, most severe first.

Correctness

1. Flaky test, red CI (above). Fix with a latch or polling appender and a thread-safe list.
2. Missing attempt guard. boundaryCallback.completed (ProxyHandler.java:1019) has no attempt == continuation.attempt check, though every other callback has one. A late completion from an abandoned attempt can overwrite continuation.request and run the old attempt's callback.
3. Entry boundary is first-wins for the whole exchange. Off-frame captures use it even for handlers positioned before the boundary. A narrow sub-pipeline boundary that runs first permanently shadows the real ExceptionHandler.
4. I/O-thread hop. Callbacks from other threads are queued behind on-thread ones. A shut-down I/O thread throws RejectedExecutionException into the caller. On the accepted-task path a failure gets a generic 500 instead of the ExceptionHandler's status and body.

Performance
5. JVM-global lock. RequestState takes synchronized(INDEX) plus a WeakHashMap lookup on every execute, capture and addHandlerMDCContext. That is about four times per proxied request, and stale-entry cleanup runs under the same lock. The state is already held by CapturedBoundary, so only off-frame lookups need the index.
6. Per-callback allocation. Each callback builds its adapter objects again. The cached adapter was removed, and no allocation numbers remain.

Cleanup
7. ProxyClientHandler.queuedRequestFailed and couldNotResolveBackend are now unreachable.
8. inherit and acceptedTask are public but only work with the private captured-policy type, so the failure is a run-time IllegalArgumentException.
9. Six rule-keyed @SuppressWarnings("java:S…") annotations hide Sonar rules instead of fixing them.

Tests
10. Nothing covers a stale-attempt completed, the index lifetime, or the entry-policy scope.

[stevehu]

Code review — round 10 (heads 05445b4 / 07fbfae)

This patch is a larger redesign. Exception policy is now an explicit Scope passed through callbacks, and proxy callbacks go through a new ordered Dispatcher. RequestState now uses a lock-free map. That resolves last round's findings on the global lock, the missing attempt guard, the first-wins entry boundary, and callback reordering from the I/O-thread hop.

SonarCloud passes on both PRs. The test job on #2836 was still pending when I checked, so I have not seen it pass or fail. I only compared the #2837 diff loosely, and it looked the same apart from README line shifts. Both PRs remain drafts.

7 findings remain, most severe first.

Correctness

1. Cleanup runs on the wrong thread. When the Dispatcher stops (fatal Error or rejected executor), rejectQueued runs onFailure (abortUpstream), the queued cleanups and scope.fail on whichever thread submits next. The proxy's non-volatile stopping, request, connection and attempt fields are meant to be I/O-thread-only.
2. Completion race in RequestState.initialize. It checks isComplete() and then calls addExchangeCompleteListener inside computeIfAbsent. Undertow throws IllegalStateException if the exchange completes in between, and the exception escapes into the late callback. Undertow's listener array is also not thread-safe, so a foreign-thread first use can race the I/O thread's own registrations.
3. Cleanup failures vanish silently. finishFailedRequest swallows every Throwable with no addSuppressed and no log. The tests that asserted suppression were weakened to verify(...).
4. Read listener ignores the captured Scope. RequestInterceptorInjectionHandler.handleEvent resumes with a plain task() call that has no policy. An exception outside resumeRead's own try block would get a bare 500 instead of the ExceptionHandler's response.

Cleanup
5. Scope and Dispatcher expose more public API than the proxy needs. Both isActive overloads, fail and the generic dispatcher(...) have no production callers. ExceptionHandler now implements HandlerWrapper, and its public wrap bypasses frames and MDC.
6. Roughly ten new objects per proxied request, plus an exchange-complete listener for every request that touches MDC, with no allocation measurement.

Tests
7. Nothing stresses the queue's multi-producer ordering, the fatal-Error path with queued events, the garbage-collected key cleanup, or the completion race.

[sonarqubecloud]

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud