You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by rs...@apache.org on 2019/01/25 00:42:59 UTC
[httpcomponents-core] branch master updated: Merge connect and
handshake timeouts in AbstractIOSessionPool
This is an automated email from the ASF dual-hosted git repository.
rschmitt 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 8f3b58d Merge connect and handshake timeouts in AbstractIOSessionPool
8f3b58d is described below
commit 8f3b58db1b1127d7a7e460d1d7912285f9628a76
Author: Ryan Schmitt <rs...@apache.org>
AuthorDate: Wed Jan 23 14:53:21 2019 -0800
Merge connect and handshake timeouts in AbstractIOSessionPool
This change essentially reverts a portion of 40cae2275 that appears to
have been unnecessary. Since the `requestTimeout` parameter is always a
connection timeout (at least in the client-side code), we don't need to
dedicate an additional parameter to the TLS handshake timeout value: we
simply use the `requestTimeout` (which has been renamed `connectTimeout`
for clarity).
---
.../nio/bootstrap/Http2MultiplexingRequester.java | 7 ++-----
.../Http2MultiplexingRequesterBootstrap.java | 10 +---------
.../apache/hc/core5/http2/nio/pool/H2ConnPool.java | 9 ++++-----
.../hc/core5/reactor/AbstractIOSessionPool.java | 17 ++++++----------
.../core5/reactor/TestAbstractIOSessionPool.java | 23 +++++++---------------
5 files changed, 20 insertions(+), 46 deletions(-)
diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/Http2MultiplexingRequester.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/Http2MultiplexingRequester.java
index 1805cd2..f3608a3 100644
--- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/Http2MultiplexingRequester.java
+++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/Http2MultiplexingRequester.java
@@ -83,7 +83,6 @@ import org.apache.hc.core5.util.Timeout;
public class Http2MultiplexingRequester extends AsyncRequester{
private final H2ConnPool connPool;
- private final Timeout handshakeTimeout;
/**
* Use {@link Http2MultiplexingRequesterBootstrap} to create instances of this class.
@@ -95,12 +94,10 @@ public class Http2MultiplexingRequester extends AsyncRequester{
final Decorator<IOSession> ioSessionDecorator,
final IOSessionListener sessionListener,
final Resolver<HttpHost, InetSocketAddress> addressResolver,
- final TlsStrategy tlsStrategy,
- final Timeout handshakeTimeout) {
+ final TlsStrategy tlsStrategy) {
super(eventHandlerFactory, ioReactorConfig, ioSessionDecorator, sessionListener,
ShutdownCommand.GRACEFUL_IMMEDIATE_CALLBACK, DefaultAddressResolver.INSTANCE);
this.connPool = new H2ConnPool(this, addressResolver, tlsStrategy);
- this.handshakeTimeout = handshakeTimeout;
}
public void closeIdle(final TimeValue idleTime) {
@@ -161,7 +158,7 @@ public class Http2MultiplexingRequester extends AsyncRequester{
throw new ProtocolException("Request authority not specified");
}
final HttpHost target = new HttpHost(scheme, authority);
- connPool.getSession(target, timeout, handshakeTimeout, new FutureCallback<IOSession>() {
+ connPool.getSession(target, timeout, new FutureCallback<IOSession>() {
@Override
public void completed(final IOSession ioSession) {
diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/Http2MultiplexingRequesterBootstrap.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/Http2MultiplexingRequesterBootstrap.java
index 589210b..2ec7801 100644
--- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/Http2MultiplexingRequesterBootstrap.java
+++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/Http2MultiplexingRequesterBootstrap.java
@@ -52,7 +52,6 @@ import org.apache.hc.core5.reactor.IOSession;
import org.apache.hc.core5.reactor.IOSessionListener;
import org.apache.hc.core5.reactor.ProtocolIOSession;
import org.apache.hc.core5.util.Args;
-import org.apache.hc.core5.util.Timeout;
/**
* {@link Http2MultiplexingRequester} bootstrap.
@@ -68,7 +67,6 @@ public class Http2MultiplexingRequesterBootstrap {
private CharCodingConfig charCodingConfig;
private H2Config h2Config;
private TlsStrategy tlsStrategy;
- private Timeout handshakeTimeout;
private boolean strictALPNHandshake;
private Decorator<IOSession> ioSessionDecorator;
private IOSessionListener sessionListener;
@@ -122,11 +120,6 @@ public class Http2MultiplexingRequesterBootstrap {
return this;
}
- public final Http2MultiplexingRequesterBootstrap setHandshakeTimeout(final Timeout handshakeTimeout) {
- this.handshakeTimeout = handshakeTimeout;
- return this;
- }
-
public final Http2MultiplexingRequesterBootstrap setStrictALPNHandshake(final boolean strictALPNHandshake) {
this.strictALPNHandshake = strictALPNHandshake;
return this;
@@ -218,8 +211,7 @@ public class Http2MultiplexingRequesterBootstrap {
ioSessionDecorator,
sessionListener,
DefaultAddressResolver.INSTANCE,
- tlsStrategy != null ? tlsStrategy : new H2ClientTlsStrategy(),
- handshakeTimeout);
+ tlsStrategy != null ? tlsStrategy : new H2ClientTlsStrategy());
}
}
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 52431a8..7f4d43f 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
@@ -97,11 +97,10 @@ public final class H2ConnPool extends AbstractIOSessionPool<HttpHost> {
@Override
protected Future<IOSession> connectSession(
final HttpHost namedEndpoint,
- final Timeout requestTimeout,
- final Timeout handshakeTimeout,
+ final Timeout connectTimeout,
final FutureCallback<IOSession> callback) {
final InetSocketAddress remoteAddress = addressResolver.resolve(namedEndpoint);
- return connectionInitiator.connect(namedEndpoint, remoteAddress, null, requestTimeout, null, new FutureCallback<IOSession>() {
+ return connectionInitiator.connect(namedEndpoint, remoteAddress, null, connectTimeout, null, new FutureCallback<IOSession>() {
@Override
public void completed(final IOSession ioSession) {
@@ -114,8 +113,8 @@ public final class H2ConnPool extends AbstractIOSessionPool<HttpHost> {
ioSession.getLocalAddress(),
ioSession.getRemoteAddress(),
null,
- handshakeTimeout);
- ioSession.setSocketTimeout(requestTimeout);
+ connectTimeout);
+ ioSession.setSocketTimeout(connectTimeout);
}
callback.completed(ioSession);
}
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 229b3b8..fa743b7 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
@@ -65,8 +65,7 @@ public abstract class AbstractIOSessionPool<T> implements ModalCloseable {
protected abstract Future<IOSession> connectSession(
T namedEndpoint,
- Timeout requestTimeout,
- final Timeout handshakeTimeout,
+ Timeout connectTimeout,
FutureCallback<IOSession> callback);
protected abstract void validateSession(
@@ -123,15 +122,13 @@ public abstract class AbstractIOSessionPool<T> implements ModalCloseable {
public final Future<IOSession> getSession(
final T endpoint,
- final Timeout requestTimeout,
- final Timeout handshakeTimeout,
+ final Timeout connectTimeout,
final FutureCallback<IOSession> callback) {
Args.notNull(endpoint, "Endpoint");
Asserts.check(!closed.get(), "Connection pool shut down");
final ComplexFuture<IOSession> future = new ComplexFuture<>(callback);
final PoolEntry poolEntry = getPoolEntry(endpoint);
- getSessionInternal(poolEntry, false, endpoint, requestTimeout, handshakeTimeout,
- new FutureCallback<IOSession>() {
+ getSessionInternal(poolEntry, false, endpoint, connectTimeout, new FutureCallback<IOSession>() {
@Override
public void completed(final IOSession ioSession) {
@@ -142,7 +139,7 @@ public abstract class AbstractIOSessionPool<T> implements ModalCloseable {
if (result) {
future.completed(ioSession);
} else {
- getSessionInternal(poolEntry, true, endpoint, requestTimeout, handshakeTimeout,
+ getSessionInternal(poolEntry, true, endpoint, connectTimeout,
new FutureCallback<IOSession>() {
@Override
@@ -185,8 +182,7 @@ public abstract class AbstractIOSessionPool<T> implements ModalCloseable {
final PoolEntry poolEntry,
final boolean requestNew,
final T namedEndpoint,
- final Timeout requestTimeout,
- final Timeout handshakeTimeout,
+ final Timeout connectTimeout,
final FutureCallback<IOSession> callback) {
synchronized (poolEntry) {
if (poolEntry.session != null && requestNew) {
@@ -203,8 +199,7 @@ public abstract class AbstractIOSessionPool<T> implements ModalCloseable {
if (poolEntry.sessionFuture == null) {
poolEntry.sessionFuture = connectSession(
namedEndpoint,
- requestTimeout,
- handshakeTimeout,
+ connectTimeout,
new FutureCallback<IOSession>() {
@Override
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 bc7ccc8..267075d 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
@@ -76,7 +76,6 @@ public class TestAbstractIOSessionPool {
Mockito.when(impl.connectSession(
ArgumentMatchers.anyString(),
ArgumentMatchers.<Timeout>any(),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.<FutureCallback<IOSession>>any())).thenReturn(connectFuture);
Mockito.doAnswer(new Answer() {
@@ -90,7 +89,7 @@ public class TestAbstractIOSessionPool {
}).when(impl).validateSession(ArgumentMatchers.<IOSession>any(), ArgumentMatchers.<Callback<Boolean>>any());
- final Future<IOSession> future1 = impl.getSession("somehost", Timeout.ofSeconds(123L), null, null);
+ final Future<IOSession> future1 = impl.getSession("somehost", Timeout.ofSeconds(123L), null);
Assert.assertThat(future1, CoreMatchers.notNullValue());
Assert.assertThat(future1.isDone(), CoreMatchers.equalTo(false));
Assert.assertThat(impl.getRoutes(), CoreMatchers.hasItem("somehost"));
@@ -98,10 +97,9 @@ public class TestAbstractIOSessionPool {
Mockito.verify(impl).connectSession(
ArgumentMatchers.eq("somehost"),
ArgumentMatchers.eq(Timeout.ofSeconds(123L)),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.<FutureCallback<IOSession>>any());
- final Future<IOSession> future2 = impl.getSession("somehost", Timeout.ofSeconds(123L), null, null);
+ 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"));
@@ -109,7 +107,6 @@ public class TestAbstractIOSessionPool {
Mockito.verify(impl, Mockito.times(1)).connectSession(
ArgumentMatchers.eq("somehost"),
ArgumentMatchers.<Timeout>any(),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.argThat(new ArgumentMatcher<FutureCallback<IOSession>>() {
@Override
@@ -128,12 +125,11 @@ public class TestAbstractIOSessionPool {
Mockito.verify(impl, Mockito.times(2)).validateSession(ArgumentMatchers.<IOSession>any(), ArgumentMatchers.<Callback<Boolean>>any());
- final Future<IOSession> future3 = impl.getSession("somehost", Timeout.ofSeconds(123L), null, null);
+ final Future<IOSession> future3 = impl.getSession("somehost", Timeout.ofSeconds(123L), null);
Mockito.verify(impl, Mockito.times(1)).connectSession(
ArgumentMatchers.eq("somehost"),
ArgumentMatchers.<Timeout>any(),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.<FutureCallback<IOSession>>any());
Mockito.verify(impl, Mockito.times(3)).validateSession(ArgumentMatchers.<IOSession>any(), ArgumentMatchers.<Callback<Boolean>>any());
@@ -148,10 +144,9 @@ public class TestAbstractIOSessionPool {
Mockito.when(impl.connectSession(
ArgumentMatchers.anyString(),
ArgumentMatchers.<Timeout>any(),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.<FutureCallback<IOSession>>any())).thenReturn(connectFuture);
- final Future<IOSession> future1 = impl.getSession("somehost", Timeout.ofSeconds(123L), null, null);
+ final Future<IOSession> future1 = impl.getSession("somehost", Timeout.ofSeconds(123L), null);
Assert.assertThat(future1, CoreMatchers.notNullValue());
Assert.assertThat(future1.isDone(), CoreMatchers.equalTo(false));
Assert.assertThat(impl.getRoutes(), CoreMatchers.hasItem("somehost"));
@@ -159,10 +154,9 @@ public class TestAbstractIOSessionPool {
Mockito.verify(impl).connectSession(
ArgumentMatchers.eq("somehost"),
ArgumentMatchers.eq(Timeout.ofSeconds(123L)),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.<FutureCallback<IOSession>>any());
- final Future<IOSession> future2 = impl.getSession("somehost", Timeout.ofSeconds(123L), null, null);
+ 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"));
@@ -170,7 +164,6 @@ public class TestAbstractIOSessionPool {
Mockito.verify(impl, Mockito.times(1)).connectSession(
ArgumentMatchers.eq("somehost"),
ArgumentMatchers.<Timeout>any(),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.argThat(new ArgumentMatcher<FutureCallback<IOSession>>() {
@Override
@@ -269,12 +262,11 @@ public class TestAbstractIOSessionPool {
}).when(impl).validateSession(ArgumentMatchers.<IOSession>any(), ArgumentMatchers.<Callback<Boolean>>any());
- impl.getSession("somehost", Timeout.ofSeconds(123L), null, null);
+ impl.getSession("somehost", Timeout.ofSeconds(123L), null);
Mockito.verify(impl, Mockito.times(1)).connectSession(
ArgumentMatchers.eq("somehost"),
ArgumentMatchers.eq(Timeout.ofSeconds(123L)),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.<FutureCallback<IOSession>>any());
}
@@ -286,12 +278,11 @@ public class TestAbstractIOSessionPool {
Mockito.when(ioSession1.isClosed()).thenReturn(true);
- impl.getSession("somehost", Timeout.ofSeconds(123L), null, null);
+ impl.getSession("somehost", Timeout.ofSeconds(123L), null);
Mockito.verify(impl).connectSession(
ArgumentMatchers.eq("somehost"),
ArgumentMatchers.eq(Timeout.ofSeconds(123L)),
- ArgumentMatchers.<Timeout>any(),
ArgumentMatchers.<FutureCallback<IOSession>>any());
}