From 5de929da6b818f8ff912c6dfb780b229d917579e Mon Sep 17 00:00:00 2001
From: Aayush Atharva <aayush@shieldblaze.com>
Date: Sat, 18 Jul 2026 11:10:55 +0000
Subject: [PATCH] Harden idle-pool reap tests: stable idle window +
 kept-after-removed coverage

Follow-up to #2225. Two test-quality fixes for the O(n) iterator-remove reap:

cleanerReapsExpiredButKeepsHealthyInSameTick used a 200ms idle window with the
fresh channels offered immediately before firing the cleaner, so a GC/scheduling
pause on a loaded CI box could age them past the timeout and get them wrongly
reaped. Widen the window to 1s (mirrors channelReofferedAfterExpiryIsNotReaped)
so only a pause longer than the window can misclassify a fresh channel.

Add cleanerContinuesPastRemovedNodesToReachKeptNodes. Because offer() is
offerFirst and that mixed test's fresh channels cluster at the front, every
reaped node sat at the tail, so the scan never had to unlink a node and then
advance to a KEPT node after it. The new test builds an alternating closed/open
deque through the remote-close path (channels are closed, not aged, so it is
fully deterministic) so each removed node is followed by a kept one, pinning the
iterator-remove-then-continue guarantee.
---
 .../netty/channel/DefaultChannelPoolTest.java | 47 ++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/client/src/test/java/org/asynchttpclient/netty/channel/DefaultChannelPoolTest.java b/client/src/test/java/org/asynchttpclient/netty/channel/DefaultChannelPoolTest.java
index 68607630c..da256149f 100644
--- a/client/src/test/java/org/asynchttpclient/netty/channel/DefaultChannelPoolTest.java
+++ b/client/src/test/java/org/asynchttpclient/netty/channel/DefaultChannelPoolTest.java
@@ -276,7 +276,10 @@ public void cleanerReapsManyIdleExpiredChannelsInOneTick() throws Exception {
     public void cleanerReapsExpiredButKeepsHealthyInSameTick() throws Exception {
         // A single reap pass must drop the expired channels AND keep the fresh ones leasable: the
         // iterator has to remove some nodes while continuing past the ones it keeps.
-        final long maxIdle = 200;
+        // Use a generous idle window (mirrors channelReofferedAfterExpiryIsNotReaped): the fresh
+        // channels are offered right before firing, so a GC/scheduling pause shorter than maxIdle
+        // cannot age them past the timeout and get them wrongly reaped on a loaded CI box.
+        final long maxIdle = 1000;
         CapturingTimer timer = new CapturingTimer();
         DefaultChannelPool pool = idlePool(timer, Duration.ofMillis(maxIdle));
 
@@ -314,6 +317,48 @@ public void cleanerReapsExpiredButKeepsHealthyInSameTick() throws Exception {
         pool.destroy();
     }
 
+    @Test
+    public void cleanerContinuesPastRemovedNodesToReachKeptNodes() throws Exception {
+        // Pins the exact iterator-remove guarantee: after unlinking a node, the scan must continue to a
+        // KEPT node that comes AFTER it in iteration order. Idle timeout is disabled (1h) so only the
+        // remote-close path trips, and channels are closed (not aged) to decide keep-vs-reap — fully
+        // deterministic, no wall-clock timing. offer() is offerFirst, so offering in reverse index order
+        // puts channels[0] at the front; the iterator then visits channels[0], channels[1], ... in order.
+        CapturingTimer timer = new CapturingTimer();
+        DefaultChannelPool pool = idlePool(timer, Duration.ofHours(1));
+
+        final int count = 8;
+        EmbeddedChannel[] channels = new EmbeddedChannel[count];
+        for (int i = count - 1; i >= 0; i--) {
+            channels[i] = new EmbeddedChannel();
+            pool.offer(channels[i], KEY);
+        }
+        // Close the even-indexed channels: in front->back iteration order every removed (even) node is
+        // immediately followed by a kept (odd) node, so the iterator must remove then advance to a keeper.
+        for (int i = 0; i < count; i += 2) {
+            channels[i].close().await(5, TimeUnit.SECONDS);
+            assertFalse(channels[i].isActive());
+        }
+
+        timer.fire();
+
+        assertEquals(count / 2, partitionSize(pool, KEY), "closed nodes unlinked, kept ones survive");
+        for (int i = 0; i < count; i++) {
+            if (i % 2 == 0) {
+                assertFalse(channels[i].isActive(), "closed channel must be unlinked");
+            } else {
+                assertTrue(channels[i].isActive(), "a kept node AFTER a removed node must survive the scan");
+            }
+        }
+        int leased = 0;
+        while (pool.poll(KEY) != null) {
+            leased++;
+        }
+        assertEquals(count / 2, leased, "every surviving channel must remain leasable");
+
+        pool.destroy();
+    }
+
     @Test
     public void cleanerUnlinksManyTombstonesInOneTick() throws Exception {
         // Many tombstones (from removeAll(Channel)) must all be unlinked in a single pass, none closed.