You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2026/10/08 18:24:38 UTC
(httpcomponents-core) 03/03: AbstractIOSessionPool: improved available sessions enumeration; added method to evict expired sessions
This is an automated email from the ASF dual-hosted git repository.
ok2c pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git
commit 07d95903168a5fb06e20321ff927204a8cfedda6
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Wed Oct 7 13:06:24 2026 +0200
AbstractIOSessionPool: improved available sessions enumeration; added method to evict expired sessions
---
.../apache/hc/core5/http2/nio/pool/H2ConnPool.java | 13 +---
.../hc/core5/reactor/AbstractIOSessionPool.java | 65 ++++++++++++++---
.../core5/reactor/TestAbstractIOSessionPool.java | 82 ++++++++++++++++++++--
3 files changed, 134 insertions(+), 26 deletions(-)
diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java
index fd6362ffe..36e2247a8 100644
--- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java
+++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/nio/pool/H2ConnPool.java
@@ -157,7 +157,7 @@ protected void validateSession(
final TimeValue inactivityTime = validateAfterInactivity;
if (TimeValue.isNonNegative(inactivityTime)) {
// Last tolerable point of inactivity
- final long deadline = inactivityDeadline(inactivityTime);
+ final long deadline = clockMillis() - inactivityTime.toMilliseconds();
if (ioSession.getLastEventTime() <= deadline) {
ioSession.enqueue(new StaleCheckCommand(callback::execute), Command.Priority.NORMAL);
return;
@@ -178,15 +178,4 @@ protected boolean isIdle(final IOSession session) {
return false;
}
- /**
- * Evict expired (closed) sessions.
- * @since 5.5
- */
- public void evictExpired() {
- // This will cause closed sessions to get removed from the pool
- enumAvailable(e -> {
- // no op
- });
- }
-
}
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/AbstractIOSessionPool.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/AbstractIOSessionPool.java
index e3acd9d52..00912a8a8 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/AbstractIOSessionPool.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/AbstractIOSessionPool.java
@@ -29,6 +29,8 @@
import java.time.Clock;
import java.util.ArrayDeque;
import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@@ -36,6 +38,7 @@
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.BiConsumer;
import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.ThreadingBehavior;
@@ -88,6 +91,13 @@ protected abstract void closeSession(
IOSession ioSession,
CloseMode closeMode);
+ /**
+ * @since 5.5
+ */
+ protected long clockMillis() {
+ return clock.millis();
+ }
+
/**
* Tests if the given session is idle. This method is expected to be communication protocol
* aware.
@@ -98,6 +108,16 @@ protected boolean isIdle(final IOSession session) {
return false;
}
+ /**
+ * Tests if the given session has expired. This method is expected to be communication protocol
+ * aware.
+ *
+ * @since 5.5
+ */
+ protected boolean isExpired(final T endpoint, final IOSession session) {
+ return false;
+ }
+
@Override
public final void close(final CloseMode closeMode) {
if (closed.compareAndSet(false, true)) {
@@ -265,13 +285,31 @@ public void cancelled() {
}
}
+ /**
+ * @deprecated Use {@link #enumAvailable(BiConsumer)}.
+ */
+ @Deprecated
public final void enumAvailable(final Callback<IOSession> callback) {
- for (final PoolEntry poolEntry: sessionPool.values()) {
+ Args.notNull(callback, "Callback");
+ enumAvailable((endpoint, session) -> callback.execute(session));
+ }
+
+ /**
+ * @since 5.5
+ */
+ public final void enumAvailable(final BiConsumer<T, IOSession> consumer) {
+ Args.notNull(consumer, "consumer");
+ for (final Iterator<Map.Entry<T, PoolEntry>> it = sessionPool.entrySet().iterator(); it.hasNext(); ) {
+ final Map.Entry<T, PoolEntry> entry = it.next();
+ final PoolEntry poolEntry = entry.getValue();
if (poolEntry.session != null) {
poolEntry.lock.lock();
try {
if (poolEntry.session != null) {
- callback.execute(poolEntry.session);
+ if (poolEntry.session.isOpen()) {
+ final T endpoint = entry.getKey();
+ consumer.accept(endpoint, poolEntry.session);
+ }
if (!poolEntry.session.isOpen()) {
poolEntry.session = null;
}
@@ -283,25 +321,32 @@ public final void enumAvailable(final Callback<IOSession> callback) {
}
}
- protected long inactivityDeadline(final TimeValue inactivityTime) {
- return this.clock.millis() - (TimeValue.isPositive(inactivityTime) ? inactivityTime.toMilliseconds() : 0);
- }
-
/**
* Close sessions idle longer than the given period of inactivity. This method will also
* Evict closed sessions.
*/
public final void closeIdle(final TimeValue inactivityTime) {
- // Last tolerable point of inactivity
- // Millisecond precision is good enough
- final long deadline = inactivityDeadline(inactivityTime);
- enumAvailable(session -> {
+ final long deadline = clockMillis() - (TimeValue.isPositive(inactivityTime) ? inactivityTime.toMilliseconds() : 0);
+ enumAvailable((endpoint, session) -> {
if (isIdle(session) && session.getLastEventTime() <= deadline) {
closeSession(session, CloseMode.GRACEFUL);
}
});
}
+ /**
+ * Evict expired (closed) sessions.
+ * @since 5.5
+ */
+ public final void evictExpired() {
+ // This method also causes closed sessions to get removed from the pool
+ enumAvailable((endpoint, session) -> {
+ if (isIdle(session) && isExpired(endpoint, session)) {
+ closeSession(session, CloseMode.GRACEFUL);
+ }
+ });
+ }
+
public final Set<T> getRoutes() {
return new HashSet<>(sessionPool.keySet());
}
diff --git a/httpcore5/src/test/java/org/apache/hc/core5/reactor/TestAbstractIOSessionPool.java b/httpcore5/src/test/java/org/apache/hc/core5/reactor/TestAbstractIOSessionPool.java
index f877da817..01851a637 100644
--- a/httpcore5/src/test/java/org/apache/hc/core5/reactor/TestAbstractIOSessionPool.java
+++ b/httpcore5/src/test/java/org/apache/hc/core5/reactor/TestAbstractIOSessionPool.java
@@ -26,7 +26,6 @@
*/
package org.apache.hc.core5.reactor;
-
import java.net.UnknownHostException;
import java.time.Clock;
import java.time.Instant;
@@ -34,6 +33,10 @@
import java.time.Month;
import java.time.ZoneId;
import java.time.ZoneOffset;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.Map;
import java.util.concurrent.Future;
import org.apache.hc.core5.concurrent.FutureCallback;
@@ -211,9 +214,32 @@ void testEnumSessions() {
Assertions.assertNotNull(entry2);
entry2.session = ioSession2;
- impl.enumAvailable(ioSession -> ioSession.close(CloseMode.GRACEFUL));
- Mockito.verify(ioSession1).close(CloseMode.GRACEFUL);
- Mockito.verify(ioSession2).close(CloseMode.GRACEFUL);
+ Mockito.doReturn(true).when(ioSession1).isOpen();
+ Mockito.doReturn(true).when(ioSession2).isOpen();
+
+ final Map<String, IOSession> sessionMap = new LinkedHashMap<>();
+ impl.enumAvailable(sessionMap::put);
+
+ Assertions.assertEquals(sessionMap.keySet(), new HashSet<>(Arrays.asList("host1", "host2")));
+ }
+
+ @Test
+ void testEnumSessionsWithClosedSession() {
+ final AbstractIOSessionPool.PoolEntry entry1 = impl.getPoolEntry("host1");
+ Assertions.assertNotNull(entry1);
+ entry1.session = ioSession1;
+
+ final AbstractIOSessionPool.PoolEntry entry2 = impl.getPoolEntry("host2");
+ Assertions.assertNotNull(entry2);
+ entry2.session = ioSession2;
+
+ Mockito.doReturn(false).when(ioSession1).isOpen();
+ Mockito.doReturn(true).when(ioSession2).isOpen();
+
+ final Map<String, IOSession> sessionMap = new LinkedHashMap<>();
+ impl.enumAvailable(sessionMap::put);
+
+ Assertions.assertEquals(sessionMap.keySet(), new HashSet<>(Arrays.asList("host2")));
}
@Test
@@ -282,9 +308,11 @@ void testCloseIdleOnly() {
Assertions.assertNotNull(entry2);
entry2.session = ioSession2;
+ Mockito.doReturn(true).when(ioSession1).isOpen();
Mockito.doReturn(true).when(impl).isIdle(ioSession1);
Mockito.doReturn(Instant.now(clock).minusSeconds(1).toEpochMilli())
.when(ioSession1).getLastEventTime();
+ Mockito.doReturn(true).when(ioSession2).isOpen();
Mockito.doReturn(false).when(impl).isIdle(ioSession2);
impl.closeIdle(null);
@@ -303,9 +331,11 @@ void testCloseIdlePastDeadline() {
Assertions.assertNotNull(entry2);
entry2.session = ioSession2;
+ Mockito.doReturn(true).when(ioSession1).isOpen();
Mockito.doReturn(true).when(impl).isIdle(ioSession1);
Mockito.doReturn(Instant.now(clock).minusSeconds(1).toEpochMilli())
.when(ioSession1).getLastEventTime();
+ Mockito.doReturn(true).when(ioSession2).isOpen();
Mockito.doReturn(true).when(impl).isIdle(ioSession2);
Mockito.doReturn(Instant.now(clock).toEpochMilli())
.when(ioSession2).getLastEventTime();
@@ -316,4 +346,48 @@ void testCloseIdlePastDeadline() {
Mockito.verify(impl, Mockito.never()).closeSession(Mockito.same(ioSession2), Mockito.any());
}
+ @Test
+ void testEvictExpired() {
+ final AbstractIOSessionPool.PoolEntry entry1 = impl.getPoolEntry("host1");
+ Assertions.assertNotNull(entry1);
+ entry1.session = ioSession1;
+
+ final AbstractIOSessionPool.PoolEntry entry2 = impl.getPoolEntry("host2");
+ Assertions.assertNotNull(entry2);
+ entry2.session = ioSession2;
+
+ Mockito.doReturn(true).when(ioSession1).isOpen();
+ Mockito.doReturn(true).when(impl).isIdle(ioSession1);
+ Mockito.doReturn(true).when(impl).isExpired("host1", ioSession1);
+ Mockito.doReturn(true).when(ioSession2).isOpen();
+ Mockito.doReturn(true).when(impl).isIdle(ioSession2);
+ Mockito.doReturn(false).when(impl).isExpired("host2", ioSession2);
+
+ impl.evictExpired();
+
+ Mockito.verify(impl).closeSession(ioSession1, CloseMode.GRACEFUL);
+ Mockito.verify(impl, Mockito.never()).closeSession(Mockito.same(ioSession2), Mockito.any());
+ }
+
+ @Test
+ void testEvictExpiredNonIdleDoNotGetEvicted() {
+ final AbstractIOSessionPool.PoolEntry entry1 = impl.getPoolEntry("host1");
+ Assertions.assertNotNull(entry1);
+ entry1.session = ioSession1;
+
+ final AbstractIOSessionPool.PoolEntry entry2 = impl.getPoolEntry("host2");
+ Assertions.assertNotNull(entry2);
+ entry2.session = ioSession2;
+
+ Mockito.doReturn(true).when(ioSession1).isOpen();
+ Mockito.doReturn(false).when(impl).isIdle(ioSession1);
+ Mockito.doReturn(true).when(ioSession2).isOpen();
+ Mockito.doReturn(false).when(impl).isIdle(ioSession2);
+
+ impl.evictExpired();
+
+ Mockito.verify(impl, Mockito.never()).closeSession(Mockito.same(ioSession1), Mockito.any());
+ Mockito.verify(impl, Mockito.never()).closeSession(Mockito.same(ioSession2), Mockito.any());
+ }
+
}