Re-arm an early read timeout for what is left - #2377

Second review round on the shared request/read entry.

Telling a stale handle from a live one by asking the scheduler whether
an entry had run was a guess on an event loop: a loop marks an entry
done only after it returns, so the code read "due" as "has run", and
a successor that was already due when it was recorded was dropped as
stale. cancel() then could not take it out. Each arming now takes a
number from a per-task counter before it schedules, and an entry is
recorded only if no later arming has begun, which says exactly what
the guess approximated.

The shared entry re-armed for what was left of the read window with
no upper bound. A wall clock stepping back made that remainder longer
than the request deadline, so the entry was armed for the deadline and
the read timeout was not checked again before it. The remainder is now
capped at a whole window, as the separate read timeout task does
since AsyncHttpClient#2377.

Also covers the read timeout starting while the entry re-arms.

Claude Code on behalf of Pavel Ptashyts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

## 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
`AtomicBoolean`s of `TimeoutsHolder` and the suspension-warning flag of
`ReadTimeoutTimerTask` become volatile ints behind static
`AtomicIntegerFieldUpdater`s. `TimeoutTimerTask.done` stays an
`AtomicBoolean`: it is a protected field of a public class, and
replacing it is the API break that closed #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. #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`. #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](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>