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 2021/09/01 20:42:36 UTC

[httpcomponents-core] branch 5.0.x updated: HTTPCORE-683: fixed incorrect recovery from hostname resolution failures by the I/O session pool

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch 5.0.x
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git


The following commit(s) were added to refs/heads/5.0.x by this push:
     new 9ce5fd1  HTTPCORE-683: fixed incorrect recovery from hostname resolution failures by the I/O session pool
9ce5fd1 is described below

commit 9ce5fd10524870d1cb1f28dd31d3a950d25e2467
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sat Aug 28 10:25:04 2021 +0200

    HTTPCORE-683: fixed incorrect recovery from hostname resolution failures by the I/O session pool
---
 .../hc/core5/reactor/AbstractIOSessionPool.java    |  5 +-
 .../core5/reactor/TestAbstractIOSessionPool.java   | 56 +++++++++++++++-------
 2 files changed, 43 insertions(+), 18 deletions(-)

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 08049be..8add086 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
@@ -196,6 +196,9 @@ public abstract class AbstractIOSessionPool<T> implements ModalCloseable {
                 callback.completed(poolEntry.session);
             } else {
                 poolEntry.requestQueue.add(callback);
+                if (poolEntry.sessionFuture != null && poolEntry.sessionFuture.isDone()) {
+                    poolEntry.sessionFuture = null;
+                }
                 if (poolEntry.sessionFuture == null) {
                     poolEntry.sessionFuture = connectSession(
                             namedEndpoint,
@@ -206,7 +209,6 @@ public abstract class AbstractIOSessionPool<T> implements ModalCloseable {
                                 public void completed(final IOSession result) {
                                     synchronized (poolEntry) {
                                         poolEntry.session = result;
-                                        poolEntry.sessionFuture = null;
                                         for (;;) {
                                             final FutureCallback<IOSession> callback = poolEntry.requestQueue.poll();
                                             if (callback != null) {
@@ -222,7 +224,6 @@ public abstract class AbstractIOSessionPool<T> implements ModalCloseable {
                                 public void failed(final Exception ex) {
                                     synchronized (poolEntry) {
                                         poolEntry.session = null;
-                                        poolEntry.sessionFuture = null;
                                         for (;;) {
                                             final FutureCallback<IOSession> callback = poolEntry.requestQueue.poll();
                                             if (callback != null) {
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 fe87e61..d926775 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,6 +26,7 @@
  */
 package org.apache.hc.core5.reactor;
 
+import java.net.UnknownHostException;
 import java.util.concurrent.Future;
 
 import org.apache.hc.core5.concurrent.FutureCallback;
@@ -34,13 +35,16 @@ import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.util.TimeValue;
 import org.apache.hc.core5.util.Timeout;
 import org.hamcrest.CoreMatchers;
+import org.hamcrest.MatcherAssert;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Answers;
+import org.mockito.ArgumentCaptor;
 import org.mockito.ArgumentMatcher;
 import org.mockito.ArgumentMatchers;
+import org.mockito.Captor;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
@@ -60,6 +64,8 @@ public class TestAbstractIOSessionPool {
     private IOSession ioSession1;
     @Mock
     private IOSession ioSession2;
+    @Captor
+    ArgumentCaptor<FutureCallback<IOSession>> connectCallbackCaptor;
 
     private AbstractIOSessionPool<String> impl;
 
@@ -141,7 +147,7 @@ public class TestAbstractIOSessionPool {
     }
 
     @Test
-    public void testGetSessionFailure() throws Exception {
+    public void testGetSessionConnectFailure() throws Exception {
 
         Mockito.when(impl.connectSession(
                 ArgumentMatchers.anyString(),
@@ -156,28 +162,20 @@ public class TestAbstractIOSessionPool {
         Mockito.verify(impl).connectSession(
                 ArgumentMatchers.eq("somehost"),
                 ArgumentMatchers.eq(Timeout.ofSeconds(123L)),
-                ArgumentMatchers.<FutureCallback<IOSession>>any());
+                connectCallbackCaptor.capture());
 
         final Future<IOSession> future2 = impl.getSession("somehost", Timeout.ofSeconds(123L), null);
         Assert.assertThat(future2, CoreMatchers.notNullValue());
         Assert.assertThat(future2.isDone(), CoreMatchers.equalTo(false));
         Assert.assertThat(impl.getRoutes(), CoreMatchers.hasItem("somehost"));
 
-        Mockito.verify(impl, Mockito.times(1)).connectSession(
-                ArgumentMatchers.eq("somehost"),
-                ArgumentMatchers.<Timeout>any(),
-                ArgumentMatchers.argThat(new ArgumentMatcher<FutureCallback<IOSession>>() {
-
-                    @Override
-                    public boolean matches(final FutureCallback<IOSession> callback) {
-                        callback.failed(new Exception("Boom"));
-                        return true;
-                    }
-
-                }));
+        final FutureCallback<IOSession> connectCallback = connectCallbackCaptor.getValue();
+        Assert.assertNotNull(connectCallback);
+        connectCallback.failed(new Exception("Boom"));
 
-        Assert.assertThat(future1.isDone(), CoreMatchers.equalTo(true));
-        Assert.assertThat(future2.isDone(), CoreMatchers.equalTo(true));
+        // Ensure connect failure invalidates all pending futures
+        MatcherAssert.assertThat(future1.isDone(), CoreMatchers.equalTo(true));
+        MatcherAssert.assertThat(future2.isDone(), CoreMatchers.equalTo(true));
     }
 
     @Test
@@ -288,4 +286,30 @@ public class TestAbstractIOSessionPool {
                 ArgumentMatchers.<FutureCallback<IOSession>>any());
     }
 
+    @Test
+    public void testGetSessionConnectUnknownHost() throws Exception {
+
+        Mockito.when(connectFuture.isDone()).thenReturn(true);
+        Mockito.when(impl.connectSession(
+                ArgumentMatchers.anyString(),
+                ArgumentMatchers.<Timeout>any(),
+                ArgumentMatchers.argThat(new ArgumentMatcher<FutureCallback<IOSession>>() {
+
+                    @Override
+                    public boolean matches(final FutureCallback<IOSession> callback) {
+                        callback.failed(new UnknownHostException("Boom"));
+                        return true;
+                    }
+
+                }))).thenReturn(connectFuture);
+
+        final Future<IOSession> future1 = impl.getSession("somehost", Timeout.ofSeconds(123L), null);
+        MatcherAssert.assertThat(future1, CoreMatchers.notNullValue());
+        MatcherAssert.assertThat(future1.isDone(), CoreMatchers.equalTo(true));
+
+        final Future<IOSession> future2 = impl.getSession("somehost", Timeout.ofSeconds(123L), null);
+        MatcherAssert.assertThat(future2, CoreMatchers.notNullValue());
+        MatcherAssert.assertThat(future2.isDone(), CoreMatchers.equalTo(true));
+    }
+
 }