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/08/28 11:40:58 UTC

[httpcomponents-core] branch HTTPCORE-683 created (now 14caf43)

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

olegk pushed a change to branch HTTPCORE-683
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git.


      at 14caf43  HTTPCORE-683: fixed incorrect recovery from hostname resolution failures by the I/O session pool

This branch includes the following new commits:

     new 14caf43  HTTPCORE-683: fixed incorrect recovery from hostname resolution failures by the I/O session pool

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[httpcomponents-core] 01/01: HTTPCORE-683: fixed incorrect recovery from hostname resolution failures by the I/O session pool

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 14caf43eae407c544161c7f92329e8beb42a3534
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   | 52 ++++++++++++++++------
 2 files changed, 41 insertions(+), 16 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 cce68d3..262d5db 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
@@ -187,6 +187,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,
@@ -197,7 +200,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) {
@@ -213,7 +215,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 83febce..695d38b 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;
@@ -35,12 +36,15 @@ 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,26 +162,18 @@ 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);
         MatcherAssert.assertThat(future2, CoreMatchers.notNullValue());
         MatcherAssert.assertThat(future2.isDone(), CoreMatchers.equalTo(false));
         MatcherAssert.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"));
 
+        // Esnure connect failure invalides all pending futures
         MatcherAssert.assertThat(future1.isDone(), CoreMatchers.equalTo(true));
         MatcherAssert.assertThat(future2.isDone(), CoreMatchers.equalTo(true));
     }
@@ -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));
+    }
+
 }