Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions client/src/main/java/org/asynchttpclient/LoadBalance.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ public enum LoadBalance {
* {@code maxConnectionsPerHost} at least as large as the resolved-IP count for full per-IP
* spreading. The default is unlimited, so this bound only applies when a finite per-host limit is
* configured.</li>
* <li>Since the per-host permit is held for the connection's lifetime, each live round-robin HTTP/2
* connection also occupies a global {@code maxConnections} slot until it closes, and pool
* {@code connectionTtl}/idle reaping do not apply (these connections live in the HTTP/2 registry,
* kept alive by PING, until GOAWAY or the connection drops). When a GOAWAY starts draining a
* connection its permit is released immediately, so a replacement can be opened without waiting
* for in-flight streams to finish.</li>
* <li>The address order comes straight from the configured
* {@link io.netty.resolver.InetNameResolver}; this mode does not re-sort it. For the
* rotation to map consistently across requests, use a resolver that returns the addresses
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,12 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
if (pk != null) {
removeHttp2Connection(pk, ctx.channel());
}
// Free the round-robin per-host permit at drain start instead of at channel close.
// A draining connection rejects new streams but can stay open while in-flight
// streams finish, and holding the permit that whole time blocks a replacement
// connection (issue #2214). Once-only: the closeFuture release becomes a no-op,
// and in DEFAULT mode no hook is installed so this does nothing.
connState.releasePermitOnce();
// Fail requests still queued for a stream slot: a draining connection accepts no
// new streams, so they can never be opened here and would otherwise wait until the
// connection finally closes. Fail them now so they retry on a fresh connection (the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;

/**
Expand Down Expand Up @@ -75,6 +76,11 @@ private static final class PendingOpener {
// opening request from acquiring a slot.
private final AtomicBoolean redundant = new AtomicBoolean(false);
private volatile Object partitionKey;
// Releases the per-host permit this connection holds in ROUND_ROBIN mode (installed by
// NettyConnectListener, absent in DEFAULT mode). The GOAWAY handler and the channel closeFuture race
// to fire it; getAndSet(null) makes it run at most once, since a double release would push the
// semaphore above maxConnectionsPerHost.
private final AtomicReference<Runnable> permitRelease = new AtomicReference<>();

public boolean tryAcquireStream() {
if (draining.get() || closed.get()) {
Expand Down Expand Up @@ -264,4 +270,24 @@ public void setPartitionKey(Object partitionKey) {
public Object getPartitionKey() {
return partitionKey;
}

/**
* Installs the action that releases this connection's per-host permit (round-robin mode only).
* {@link #releasePermitOnce()} runs it at most once.
*/
public void setPermitRelease(Runnable release) {
permitRelease.set(release);
}

/**
* Runs the installed permit-release action the first time it is called; later calls do nothing, as do
* calls when no action was installed (DEFAULT mode). Fired at drain start (GOAWAY) and on channel
* close, whichever comes first.
*/
public void releasePermitOnce() {
Runnable release = permitRelease.getAndSet(null);
if (release != null) {
release.run();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +283,23 @@ private void releaseSemaphoreImmediately(Object partitionKeyLock) {
*/
private void registerHttp2AndManageSemaphore(Channel channel, Object partitionKeyLock) {
// Register under the future's partition key so the H2 connection is found by the same key the
// pool is polled with — including the IP-aware key used by LoadBalance.ROUND_ROBIN.
channelManager.registerHttp2Connection(future.getPartitionKey(), channel);
if (future.getPartitionKey() instanceof RoundRobinPartitionKey) {
attachSemaphoreToChannelClose(channel, partitionKeyLock);
// pool is polled with, including the IP-aware key used by LoadBalance.ROUND_ROBIN. Read the key
// once: it is volatile (repinned on IP failover) and both uses below must agree.
Object partitionKey = future.getPartitionKey();
channelManager.registerHttp2Connection(partitionKey, channel);
if (partitionKey instanceof RoundRobinPartitionKey) {
// The permit must be freed as soon as the connection stops serving new requests: on close, or
// at drain start after GOAWAY (issue #2214). Both paths go through releasePermitOnce(), which
// guarantees a single release; a double release would push the semaphore above the cap. The
// state attribute is always present after upgradePipelineToHttp2, the fallback is just
// belt and braces.
Http2ConnectionState state = channel.attr(Http2ConnectionState.HTTP2_STATE_KEY).get();
if (state != null && connectionSemaphore != null && partitionKeyLock != null) {
state.setPermitRelease(() -> connectionSemaphore.releaseChannelLock(partitionKeyLock));
channel.closeFuture().addListener(f -> state.releasePermitOnce());
} else {
attachSemaphoreToChannelClose(channel, partitionKeyLock);
}
} else {
releaseSemaphoreImmediately(partitionKeyLock);
}
Expand Down
44 changes: 44 additions & 0 deletions client/src/test/java/org/asynchttpclient/BasicHttp2Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,50 @@ public void http2RoundRobinCapsLiveConnectionsAtMaxConnectionsPerHost() throws E
}
}

/**
* Issue #2214 drain-permit fix: with {@code maxConnectionsPerHost=1} a single live round-robin
* connection saturates the per-host cap, so its permit must be released when the server's GOAWAY starts
* draining it, not when it finally closes. The first request is a long-running stream that keeps the
* connection open across the drain (the GOAWAY carries a high lastStreamId so the stream survives it,
* and the request is never awaited); the second request must then open a replacement connection promptly
* instead of failing with {@code TooManyConnectionsPerHostException} after stalling for
* {@code connectTimeout}.
*/
@Test
public void http2RoundRobinGoawayReleasesPermitForReplacementConnection() throws Exception {
io.netty.resolver.NameResolver<java.net.InetAddress> resolver = multiIpResolver("127.0.0.1", "127.0.0.2");
try (AsyncHttpClient client = http2ClientWithConfig(b -> b.setLoadBalance(LoadBalance.ROUND_ROBIN)
.setMaxConnectionsPerHost(1).setMaxRequestRetry(0)
.setConnectTimeout(Duration.ofSeconds(1)).setRequestTimeout(Duration.ofSeconds(60)))) {

// Long-running stream that keeps its connection (and, before the fix, the only permit) busy.
client.executeRequest(org.asynchttpclient.Dsl.get(httpsUrl("/delay/30000")).setNameResolver(resolver));

// Wait until the server has accepted the connection.
long deadline = System.currentTimeMillis() + 5000;
while (serverChildChannels.size() < 1 && System.currentTimeMillis() < deadline) {
Thread.sleep(20);
}
assertEquals(1, serverChildChannels.size(), "exactly one HTTP/2 connection should be established");
Thread.sleep(300);

// GOAWAY with a high lastStreamId leaves the in-flight stream running, so the connection
// stays open and draining.
Channel parent = serverChildChannels.iterator().next();
parent.writeAndFlush(new io.netty.handler.codec.http2.DefaultHttp2GoAwayFrame(Http2Error.NO_ERROR)
.setExtraStreamIds(1000)).sync();
Thread.sleep(300);

// Must open a replacement connection; before the fix this failed with
// TooManyConnectionsPerHostException because the draining connection still held the permit.
Response replacement = client.executeRequest(
org.asynchttpclient.Dsl.get(httpsUrl("/ok")).setNameResolver(resolver)).get(10, SECONDS);
assertEquals(200, replacement.getStatusCode(),
"a request after GOAWAY must open a replacement connection, not fail with "
+ "TooManyConnectionsPerHostException while the draining connection pins the permit");
}
}

/**
* Creates an AHC client with a specific request timeout.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
/*
* Copyright (c) 2026 AsyncHttpClient Project. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.asynchttpclient.netty.channel;

import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.util.HashedWheelTimer;
import io.netty.util.Timer;
import org.asynchttpclient.AsyncHttpClientConfig;
import org.asynchttpclient.exception.TooManyConnectionsException;
import org.asynchttpclient.exception.TooManyConnectionsPerHostException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.net.InetAddress;

import static org.asynchttpclient.Dsl.config;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* Regression tests for the round-robin GOAWAY drain-permit fix (issue #2214): the per-host permit held for
* an HTTP/2 connection's lifetime must be released when the connection starts draining, not when it finally
* closes, and released exactly once across the GOAWAY path and the channel closeFuture. The wiring mirrors
* {@link NettyConnectListener}'s round-robin branch and {@link ChannelManager}'s GOAWAY handler, without
* network I/O; the end-to-end path over a real socket is covered by {@code BasicHttp2Test}.
*/
class ChannelManagerHttp2DrainPermitTest {

private static final String BASE = "https://host:443";

private ChannelManager channelManager;
private Timer timer;

@BeforeEach
void setUp() {
AsyncHttpClientConfig cfg = config().build();
timer = new HashedWheelTimer();
channelManager = new ChannelManager(cfg, timer);
}

@AfterEach
void tearDown() {
if (channelManager != null) {
channelManager.close();
}
if (timer != null) {
timer.stop();
}
}

private static RoundRobinPartitionKey rrKey(String base, String ip) throws Exception {
return new RoundRobinPartitionKey(base, InetAddress.getByName(ip));
}

/**
* Registers an active round-robin HTTP/2 connection holding a per-host permit, wired as
* {@link NettyConnectListener} does: permit keyed by the base key, connection registered under the
* IP-aware key, release hook plus closeFuture listener installed.
*/
private EmbeddedChannel registerRrConnectionHoldingPermit(ConnectionSemaphore semaphore, Object baseKey,
Object registryKey) {
EmbeddedChannel channel = new EmbeddedChannel();
Http2ConnectionState state = new Http2ConnectionState();
channel.attr(Http2ConnectionState.HTTP2_STATE_KEY).set(state);
channelManager.registerHttp2Connection(registryKey, channel);
// Mirror NettyConnectListener.registerHttp2AndManageSemaphore's round-robin branch.
state.setPermitRelease(() -> semaphore.releaseChannelLock(baseKey));
channel.closeFuture().addListener(f -> state.releasePermitOnce());
return channel;
}

/** Available per-host permits for {@code baseKey} under a {@link PerHostConnectionSemaphore}. */
private static int availablePerHost(PerHostConnectionSemaphore semaphore, Object baseKey) {
return semaphore.getFreeConnectionsForHost(baseKey).availablePermits();
}

/**
* GOAWAY on a still-open connection must free the per-host permit at drain start, and the later
* channel close must not release a second one.
*/
@Test
void goawayReleasesPerHostPermitImmediatelyAndCloseDoesNotDoubleRelease() throws Exception {
PerHostConnectionSemaphore semaphore = new PerHostConnectionSemaphore(1, 0);
RoundRobinPartitionKey registryKey = rrKey(BASE, "127.0.0.1");
semaphore.acquireChannelLock(BASE);
assertEquals(0, availablePerHost(semaphore, BASE), "permit is held after acquire");

EmbeddedChannel channel = registerRrConnectionHoldingPermit(semaphore, BASE, registryKey);
Http2ConnectionState state = channel.attr(Http2ConnectionState.HTTP2_STATE_KEY).get();
// An in-flight stream keeps the connection open through the drain.
assertTrue(state.tryAcquireStream());

// Simulate GOAWAY exactly as ChannelManager's handler does.
state.setDraining(0);
channelManager.removeHttp2Connection(state.getPartitionKey(), channel);
state.releasePermitOnce();

assertEquals(1, availablePerHost(semaphore, BASE),
"the per-host permit must be freed at drain start, while the channel is still open");
assertTrue(channel.isOpen(), "the draining connection is still open (its stream is in flight)");
assertNull(channelManager.pollHttp2SiblingConnection(BASE), "a draining connection is not offered for reuse");

// The in-flight stream ends and the channel closes: closeFuture must NOT release a second permit.
state.releaseStream();
channel.close().sync();
assertEquals(1, availablePerHost(semaphore, BASE),
"channel close after a GOAWAY drain must not release a second permit (never exceed the cap)");
}

/**
* A connection that closes without ever receiving a GOAWAY (normal drop, TCP reset) must still release
* its permit exactly once, via the closeFuture; a stray later releasePermitOnce is a no-op.
*/
@Test
void closeWithoutGoawayReleasesPermitExactlyOnce() throws Exception {
PerHostConnectionSemaphore semaphore = new PerHostConnectionSemaphore(1, 0);
RoundRobinPartitionKey registryKey = rrKey(BASE, "127.0.0.1");
semaphore.acquireChannelLock(BASE);

EmbeddedChannel channel = registerRrConnectionHoldingPermit(semaphore, BASE, registryKey);
Http2ConnectionState state = channel.attr(Http2ConnectionState.HTTP2_STATE_KEY).get();
assertEquals(0, availablePerHost(semaphore, BASE));

channel.close().sync();
assertEquals(1, availablePerHost(semaphore, BASE), "close releases the permit once");

// A late GOAWAY-style release (or any extra call) must not push the semaphore above the cap.
state.releasePermitOnce();
assertEquals(1, availablePerHost(semaphore, BASE), "a second release must be a no-op");
}

/**
* With a per-host cap of 1, a new connection to the same host must be admissible again once the drain
* releases the permit; without the fix the draining connection pins the only permit and a replacement
* fails with {@link TooManyConnectionsPerHostException}.
*/
@Test
void replacementConnectionIsAdmissibleAfterDrainRelease() throws Exception {
PerHostConnectionSemaphore semaphore = new PerHostConnectionSemaphore(1, 0);
RoundRobinPartitionKey registryKey = rrKey(BASE, "127.0.0.1");
semaphore.acquireChannelLock(BASE);

EmbeddedChannel channel = registerRrConnectionHoldingPermit(semaphore, BASE, registryKey);
Http2ConnectionState state = channel.attr(Http2ConnectionState.HTTP2_STATE_KEY).get();
assertTrue(state.tryAcquireStream()); // keep it open through the drain

// Before the fix, the permit is still held here, so this acquire fails.
assertThrows(TooManyConnectionsPerHostException.class, () -> semaphore.acquireChannelLock(BASE),
"sanity: the cap is exhausted while the connection holds its permit");

// GOAWAY drain releases the permit.
state.setDraining(0);
channelManager.removeHttp2Connection(state.getPartitionKey(), channel);
state.releasePermitOnce();

// A replacement connection can now be opened immediately, without waiting for the drain to finish.
assertDoesNotThrow(() -> semaphore.acquireChannelLock(BASE),
"after the drain release a replacement connection must be admissible");

state.releaseStream();
channel.close().sync();
}

/**
* With a combined limiter the round-robin permit pins both a per-host and a global slot. Releasing it
* at drain start must free the global slot too, so unrelated hosts are not starved.
*/
@Test
void goawayReleasesGlobalSlotSoUnrelatedHostIsNotStarved() throws Exception {
// Global cap 1, per-host cap 1: a single live round-robin connection saturates the global limit.
CombinedConnectionSemaphore semaphore = new CombinedConnectionSemaphore(1, 1, 0);
Object otherHost = "https://other:443";
RoundRobinPartitionKey registryKey = rrKey(BASE, "127.0.0.1");
semaphore.acquireChannelLock(BASE);

EmbeddedChannel channel = registerRrConnectionHoldingPermit(semaphore, BASE, registryKey);
Http2ConnectionState state = channel.attr(Http2ConnectionState.HTTP2_STATE_KEY).get();
assertTrue(state.tryAcquireStream()); // keep it open through the drain

// The global slot is pinned: even an unrelated host cannot connect.
assertThrows(TooManyConnectionsException.class, () -> semaphore.acquireChannelLock(otherHost),
"sanity: the global cap is exhausted while the draining connection holds its slot");

// GOAWAY drain releases the combined (global + per-host) permit.
state.setDraining(0);
channelManager.removeHttp2Connection(state.getPartitionKey(), channel);
state.releasePermitOnce();

assertDoesNotThrow(() -> semaphore.acquireChannelLock(otherHost),
"releasing the draining connection's permit must free the global slot for an unrelated host");

state.releaseStream();
channel.close().sync();
}
}
Loading
Loading