Skip to content

Wait for a sibling HTTP/2 connection without blocking the caller thread#2227

Merged
hyperxpro merged 5 commits into
AsyncHttpClient:mainfrom
maygemdev:fix/h2-wait-no-caller-thread-spin
Jul 18, 2026
Merged

Wait for a sibling HTTP/2 connection without blocking the caller thread#2227
hyperxpro merged 5 commits into
AsyncHttpClient:mainfrom
maygemdev:fix/h2-wait-no-caller-thread-spin

Conversation

@pavel-ptashyts

Copy link
Copy Markdown
Contributor

When the per-host connection cap is saturated and HTTP/2 is enabled, a request that fails to acquire a permit tries to multiplex onto a sibling connection another request is establishing to the same origin (stream reuse needs no permit). Off the event loop, waitForHttp2Connection did this by Thread.sleep(10)-polling the H2 registry until connectTimeout (5s default) elapsed — parking the caller thread (the synchronous part of execute()) for up to the full timeout and burning CPU. Under a bounded caller thread pool, a burst of over-cap requests to a new H2 origin could exhaust the pool.

Replace the busy-poll with an event-driven, non-blocking deferral: register a one-shot waiter keyed by the request's partition key and return the pending future immediately. registerHttp2Connection wakes matching waiters, which resume the send via sendRequestWithOpenChannel. A connectTimeout deadline on the Netty timer fails the request with the original permit exception if no connection arrives; the client-close path fails pending waiters (their request-timeout backstop is not scheduled yet). A once-only CAS makes the registration, deadline, and close paths mutually exclusive, and the waiter rechecks the registry after registering to close the poll-vs-register lost-wakeup race.

On the event loop (a redirect / 401 / 407 retry) the single immediate poll is kept and we give up if it misses — a wait there could self-deadlock, since the connection is being established on that same loop. The ROUND_ROBIN #2214 limitation is unchanged (per-IP keying) but no longer occupies the caller thread.

Adds ChannelManagerHttp2WaiterTest covering wake-on-registration, key isolation, waiter removal, and fail-on-close. Existing HTTP/2 regression tests (conformance, multiplexing, stream-orphan, streaming-body flow-control) pass unchanged.

When the per-host connection cap is saturated and HTTP/2 is enabled, a request that fails to acquire a permit tries to multiplex onto a sibling connection another request is establishing to the same origin (stream reuse needs no permit). Off the event loop, waitForHttp2Connection did this by Thread.sleep(10)-polling the H2 registry until connectTimeout (5s default) elapsed — parking the caller thread (the synchronous part of execute()) for up to the full timeout and burning CPU. Under a bounded caller thread pool, a burst of over-cap requests to a new H2 origin could exhaust the pool.

Replace the busy-poll with an event-driven, non-blocking deferral: register a one-shot waiter keyed by the request's partition key and return the pending future immediately. registerHttp2Connection wakes matching waiters, which resume the send via sendRequestWithOpenChannel. A connectTimeout deadline on the Netty timer fails the request with the original permit exception if no connection arrives; the client-close path fails pending waiters (their request-timeout backstop is not scheduled yet). A once-only CAS makes the registration, deadline, and close paths mutually exclusive, and the waiter rechecks the registry after registering to close the poll-vs-register lost-wakeup race.

On the event loop (a redirect / 401 / 407 retry) the single immediate poll is kept and we give up if it misses — a wait there could self-deadlock, since the connection is being established on that same loop. The ROUND_ROBIN AsyncHttpClient#2214 limitation is unchanged (per-IP keying) but no longer occupies the caller thread.

Adds ChannelManagerHttp2WaiterTest covering wake-on-registration, key isolation, waiter removal, and fail-on-close. Existing HTTP/2 regression tests (conformance, multiplexing, stream-orphan, streaming-body flow-control) pass unchanged.
…-thread-spin

# Conflicts:
#	client/src/main/java/org/asynchttpclient/netty/channel/ChannelManager.java
#	client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java
…gs reuse

A permit-starved request that deferred onto an Http2ConnectionWaiter was keyed
by its full per-IP partition key, so in LoadBalance.ROUND_ROBIN mode it was only
woken by a connection registered for its own pinned IP — never by a sibling-IP
connection to the same host that finished connecting later. It then waited out
connectTimeout and failed with TooManyConnectionsPerHostException, regressing the
issue AsyncHttpClient#2214 sibling-reuse behaviour that main's earlier poll-loop provided.

Group waiters in ChannelManager by the per-host base key (baseKeyOf) instead of
the full partition key, so a registration for any IP of the host wakes them and
they multiplex onto that sibling connection. Non-round-robin keys are unaffected
(baseKeyOf is identity for non-RoundRobinPartitionKey).

Fixes BasicHttp2Test.http2RoundRobinPermitStarvedReusesSiblingConnection.
@hyperxpro
hyperxpro merged commit dd44e61 into AsyncHttpClient:main Jul 18, 2026
13 checks passed
hyperxpro added a commit that referenced this pull request Jul 18, 2026
…and shutdown races (#2251)

Motivation:

#2227 introduced an event-driven HTTP/2 connection-waiter registry in
`ChannelManager`, but three issues remained. A waiter throwing an
exception could prevent subsequent waiters from being notified and
interrupt the connection establishment path, leaking a permit and
leaving the request hanging. Requests registering during shutdown could
miss both the close notification and timeout, leaving their futures
incomplete. Additionally, timed-out or removed waiters left empty
per-host sets behind for the lifetime of the client.

Modification:

Isolate waiter callbacks in a `notifyHttp2ConnectionWaiter` try/catch
helper. Add a `waitersClosed` flag that is latched before the shutdown
sweep so new waiters are rejected during shutdown, allowing `arm()` to
fail the request immediately without interacting with the timer. Remove
empty waiter sets using `computeIfPresent`. Added four unit tests
covering these scenarios.

Result:

Misbehaving waiters can no longer disrupt connection establishment or
leak permits, waiters registered during shutdown fail deterministically
instead of hanging, and the registry no longer retains empty per-host
entries.
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.

2 participants