[pavel-ptashyts]

Summary

Every attempt builds a TimeoutsHolder, arms its request timeout and, once the request is written, its read timeout, then cancels both when the exchange ends. When the read timeout is shorter than the request timeout, that is two timer entries per attempt, and almost every exchange cancels both unfired. On the client's HashedWheelTimer each entry is an increment of a client-wide AtomicLong and an offer to a client-wide MPSC queue, each cancel an offer to a second one, and the timer's single thread drains them all. With useEventLoopTimeouts each is a scheduled task added to and removed from the loop.

Changes:

1. RequestTimeoutsArmBenchmark measures one attempt's timeout lifecycle on a real HashedWheelTimer, with the read timeout shorter and not shorter than the request timeout. RequestTimeoutsFireBenchmark measures an exchange that outlives its read window: the entry that checks the read timeout fires one or five times, finding a recent read and arming again each time.
2. Private timeout guards in field updaters. The two AtomicBooleans of TimeoutsHolder and the suspension-warning flag of ReadTimeoutTimerTask become volatile ints behind static AtomicIntegerFieldUpdaters. TimeoutTimerTask.done stays an AtomicBoolean: it is a protected field of a public class, and replacing it is the API break that closed Use field updaters for timeout guards #2263.
3. One timer entry per attempt. When the read timeout is the shorter one, start() arms the request timeout's entry for when a read timeout would be due. Once the read timeout starts it rides on that entry instead of taking its own. On firing, the entry fails the exchange with a read timeout when one is due, and otherwise re-arms for whichever deadline comes next: what is left of the read window, capped at a whole window and by the request deadline.
4. Fixes from the review rounds below, each with a test that fails without it:
   • the request deadline that the shared entry re-arms and checks against is kept on the monotonic clock (an absolute deadline straight off the exchange's start), so a wall clock step cannot move it;
   • each arming takes a number from a per-task counter, and an entry is recorded only if no later arming has begun, so an arming that loses the race against its own firing cannot hide the newer entry from cancel(); the handle is one field, taken atomically on cancel;
   • if the timer rejects the re-armed entry (a user timer that is full or stopped), the exchange is failed instead of being left with no deadline, but only while this holder is still the future's and the future is not done;
   • the monotonic clock is read only where the deadline is needed (an entry armed early, or useEventLoopTimeouts), so the default attempt costs no more than on main;
   • an entry that fires early reads the monotonic clock once, both to check the request deadline and to time the next entry; it skips the read-state CAS once the read timeout is covered, and writes the early flag only when the flag changes. Without these, five fires per exchange ran slower than main's separate read entry.

Behaviour notes

• Same timeouts, same messages, same exceptions.
• A read timeout that has not started when the shared entry fires (a slow connect or upload) goes back to an entry of its own, as before. That costs one fire and re-arm that main does not have.
• An exchange that outlives its read window re-arms the shared entry for what is left of the window, capped at a whole window. Re-arm an early read timeout for what is left #2377 re-arms main's separate read entry the same way. Each fire arms one entry, as each fire of the read entry does on main, and the request timeout's own entry is gone.
• An entry that fires past the request deadline (a wheel tick can land after it) delivers the request timeout.
• A suspended response body still holds off the read timeout for a whole window, and the indefinite-suspension warning is unchanged.
• Absolute deadlines, rehomeOn, useEventLoopTimeouts and cancel without pinning the future are kept; their existing tests pass unchanged.
• No public API change: Revapi passes. New members are private or package-private; TimeoutsHolder.cancel() keeps its signature.

Relation to #2377

#2377 makes the separate read timeout entry re-arm for what is left of its window rather than a whole one. It has since merged into main. This branch still merges into main cleanly (git merge-tree against f5ad541), and no merge of main was pushed here. Before #2377 merged, both PRs' timeout tests passed on the merged tree. The shared entry here re-arms for the remaining window the same way, capped at a whole window as #2377 caps it.

Benchmark

RequestTimeoutsArmBenchmark.armAndCancel, request timeout 60 s. JMH 1.37, JDK 17.0.2, Windows, -prof gc, 3 forks, 3 x 2 s warmup, 5 x 2 s measurement, main against 12291df. The later commits change the firing path; on the arming path they only skip a write of a flag that did not change.

Read timeout 10 s (two entries on main, one here):

Threads	main (ops/us)	This PR (ops/us)	Bytes/op
1	2.76 +- 0.02	3.57 +- 0.03	391 -> 223
8	5.35 +- 1.13	10.71 +- 3.11	392 -> 225
32	1.74 +- 0.52	3.36 +- 0.96	395 -> 226

Read timeout 60 s, the default (one entry either way):

Threads	main (ops/us)	This PR (ops/us)	Bytes/op
1	3.99 +- 0.03	3.97 +- 0.02	247 -> 223
8	7.43 +- 4.59	11.08 +- 2.89	249 -> 225
32	3.91 +- 1.05	4.32 +- 1.07	249 -> 226

RequestTimeoutsFireBenchmark.armFireAndCancel, read timeout 10 s, request timeout 60 s, same settings, main against 44b7bc3. The benchmark fires the entry from its own thread by taking it out of the wheel (a cancel) and running its task. Both builds pay that cancel once per fire. At one thread, main and this branch ran alternately, twice each:

Threads	Fires	main (ops/us)	This PR (ops/us)	Bytes/op
1	1	1.67 +- 0.32, 1.71 +- 0.32	1.99 +- 0.33, 1.88 +- 0.34	455 -> 287
1	5	0.76 +- 0.20, 0.61 +- 0.12	0.99 +- 0.10, 0.89 +- 0.14	709 -> 541
8	1	1.71 +- 1.20	5.07 +- 0.82	459 -> 290
8	5	1.27 +- 0.17	1.31 +- 0.45	716 -> 550
32	1	1.23 +- 0.39	1.44 +- 0.53	460 -> 292
32	5	0.51 +- 0.20	0.67 +- 0.27	722 -> 553

The host was shared with other builds while these ran, so the multi-threaded rows have wide error bars; allocation is the reliable figure there.

Tests

New RequestTimeoutTimerTaskTest (26 tests) drives a holder with fake wall and monotonic clocks and a recording timer and event loop. It covers one entry serving both timeouts, re-arming for the rest of the read window, delivery of read and request timeouts from the shared entry, a read timeout that starts late or while the entry re-arms, absolute deadlines, wall clock steps both ways, suspended bodies, rehomeOn before the read timeout starts, cancellation after an early fire on the timer and on a loop, an arming that loses the race to its own firing, a re-arm the timer rejects (with a live, a replaced and a detached holder), a request timeout of Long.MAX_VALUE ms whose deadline wraps on the monotonic clock, a completed exchange fired inside its read window, the default attempt not reading the monotonic clock, and an early fire reading it once.

./mvnw clean verify -Dgpg.skip=true passes on JDK 17 (1965 tests; no JDK 11 and no gpg on the host); CI covers JDK 11.

Maintainer review

Eight comments from hyperxpro, written against c8df6c3. At 12291df, five were already fixed: the deadline on the monotonic clock with subtraction, the one-window cap, the Long.MAX_VALUE overflow, the atomic take of the handle in cancelArmed(), and the abort on a rejected re-arm. The done.set(false) ordering test already existed (aReadTimeoutStartingWhileTheEntryReArmsGetsItsOwnEntry). This push adds the rest: the overflow test, the completed-exchange test fired inside the window, RequestTimeoutsFireBenchmark, and the per-fire trims it led to. Both new review tests fail when the code they guard is reverted, and so does the existing ordering test when done.set(false) moves below the re-arm.

Second-agent review

Both reviewers ran from fresh CLI sessions, read-only, on git diff upstream/main...HEAD, with the intent and constraints in the prompt.

Reviewer	Agent and model	Effort	Commit reviewed	Result
Guided (against AGENTS.md)	Codex gpt-6-astra	medium	12291df	No actionable findings
Independent (no checklist)	Claude Code claude-opus-5-5	medium	12291df	No actionable findings; two optional notes, no change made

Seven rounds in all (c8df6c3, f2cddcc, 3daa3b7, cbb6a78, fe9fc58, e678f09, 12291df). Every actionable finding of the earlier rounds was fixed in the commits listed under change 4, except these, which were not changed:

• startReadTimeout(task) still compares the read timeout against the request deadline on the wall clock. Inherited from main. Re-arm an early read timeout for what is left #2377, now merged, rewrote the same line. Changing it here would need main merged into this branch, so it is left for a follow-up after this PR.
• Check-then-act window when a rejected re-arm aborts the exchange. After the ownership check passes, a redirect or completion can still replace the holder or pool the channel before abort runs. Every timeout delivery on main has the same window (expire() checks and then aborts with the future's current channel from the timer thread); closing it means serializing timeout delivery with attempt replacement on the channel's event loop, a redesign outside this PR. The path is reached only when a user-supplied timer rejects an entry.
• start() sets the pending read state with a plain write (round 6, independent reviewer, low). A read timeout started on a new holder before start() runs can leave both a separate read entry and the early entry; main double-arms the read timeout in the same race, and the two entries run on the same scheduler, so the overlap that would let one drop the other does not arise outside a rehomeOn.
• rehomeOn javadoc still says it is a no-op once the request timeout has fired; an early firing of the shared entry is not that, and the next re-arm reads the new executor anyway. Wording only.

Follow-up idea, not in this PR

A per-connection lazy deadline for pooled HTTP/1.1 connections, in the style of IdleStateHandler: one entry per connection, re-armed only when it fires early or a new deadline is earlier, so a keep-alive request touches neither the timer nor a TimeoutsHolder. A throwaway prototype put the per-request cost near zero, but it is a new synchronization protocol at the exchange handover on a pooled channel, so it should wait for an end-to-end keep-alive benchmark that shows how much of a whole request this cost is.

Claude Code on behalf of Pavel Ptashyts

🤖 Generated with Claude Code