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:29:22 UTC
[httpcomponents-core] branch master 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 master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git
The following commit(s) were added to refs/heads/master by this push:
new 399fd2c HTTPCORE-683: fixed incorrect recovery from hostname resolution failures by the I/O session pool
399fd2c is described below
commit 399fd2ccdc2ab95fef7a58930e5912c24180f6e6
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 | 42 +++++++++++++++++-----
2 files changed, 36 insertions(+), 11 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 2bc396a..efe855d 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
@@ -182,6 +182,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,
@@ -192,7 +195,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) {
@@ -208,7 +210,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 04f3dd4..c1f1672 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,11 +36,14 @@ 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.ArgumentMatchers;
+import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
@@ -57,6 +61,8 @@ public class TestAbstractIOSessionPool {
private IOSession ioSession1;
@Mock
private IOSession ioSession2;
+ @Captor
+ ArgumentCaptor<FutureCallback<IOSession>> connectCallbackCaptor;
private AbstractIOSessionPool<String> impl;
@@ -128,7 +134,7 @@ public class TestAbstractIOSessionPool {
}
@Test
- public void testGetSessionFailure() throws Exception {
+ public void testGetSessionConnectFailure() throws Exception {
Mockito.when(impl.connectSession(
ArgumentMatchers.anyString(),
@@ -143,21 +149,18 @@ public class TestAbstractIOSessionPool {
Mockito.verify(impl).connectSession(
ArgumentMatchers.eq("somehost"),
ArgumentMatchers.eq(Timeout.ofSeconds(123L)),
- ArgumentMatchers.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.any(),
- ArgumentMatchers.argThat(callback -> {
- callback.failed(new Exception("Boom"));
- return true;
- }));
+ final FutureCallback<IOSession> connectCallback = connectCallbackCaptor.getValue();
+ Assert.assertNotNull(connectCallback);
+ connectCallback.failed(new Exception("Boom"));
+ // Ensure connect failure invalidates all pending futures
MatcherAssert.assertThat(future1.isDone(), CoreMatchers.equalTo(true));
MatcherAssert.assertThat(future2.isDone(), CoreMatchers.equalTo(true));
}
@@ -258,4 +261,25 @@ public class TestAbstractIOSessionPool {
ArgumentMatchers.any());
}
+ @Test
+ public void testGetSessionConnectUnknownHost() throws Exception {
+
+ Mockito.when(connectFuture.isDone()).thenReturn(true);
+ Mockito.when(impl.connectSession(
+ ArgumentMatchers.anyString(),
+ ArgumentMatchers.any(),
+ ArgumentMatchers.argThat(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));
+ }
+
}