Skip to content

Commit 05042bc

Browse files
arturobernalgarturobernalg
committed
Off-lock disposal in PoolingHttpClientConnectionManager: enqueue GRACEFUL, run IMMEDIATE inline
Drain queue after lease/release/closeExpired/closeIdle to avoid holding the pool lock
1 parent 830b062 commit 05042bc

5 files changed

Lines changed: 485 additions & 8 deletions

File tree

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/OffLockDisposalCallback.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* ====================================================================
3+
* Licensed to the Apache Software Foundation (ASF) under one
4+
* or more contributor license agreements. See the NOTICE file
5+
* distributed with this work for additional information
6+
* regarding copyright ownership. The ASF licenses this file
7+
* to you under the Apache License, Version 2.0 (the
8+
* "License"); you may not use this file except in compliance
9+
* with the License. You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing,
14+
* software distributed under the License is distributed on an
15+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
* KIND, either express or implied. See the License for the
17+
* specific language governing permissions and limitations
18+
* under the License.
19+
* ====================================================================
20+
*
21+
* This software consists of voluntary contributions made by many
22+
* individuals on behalf of the Apache Software Foundation. For more
23+
* information on the Apache Software Foundation, please see
24+
* <http://www.apache.org/>.
25+
*
26+
*/
27+
package org.apache.hc.client5.http.impl.io;
28+
29+
import java.util.Queue;
30+
import java.util.concurrent.ConcurrentLinkedQueue;
31+
32+
import org.apache.hc.core5.annotation.Internal;
33+
import org.apache.hc.core5.io.CloseMode;
34+
import org.apache.hc.core5.io.ModalCloseable;
35+
import org.apache.hc.core5.pool.DisposalCallback;
36+
37+
@Internal
38+
final class OffLockDisposalCallback<T extends ModalCloseable> implements DisposalCallback<T> {
39+
40+
private final DisposalCallback<T> delegate;
41+
private final Queue<T> gracefulQueue = new ConcurrentLinkedQueue<>();
42+
43+
OffLockDisposalCallback(final DisposalCallback<T> delegate) {
44+
this.delegate = delegate;
45+
}
46+
47+
@Override
48+
public void execute(final T closeable, final CloseMode mode) {
49+
if (mode == CloseMode.IMMEDIATE) {
50+
delegate.execute(closeable, CloseMode.IMMEDIATE);
51+
} else {
52+
gracefulQueue.offer(closeable);
53+
}
54+
}
55+
56+
void drain() {
57+
for (T c; (c = gracefulQueue.poll()) != null; ) {
58+
delegate.execute(c, CloseMode.GRACEFUL);
59+
}
60+
}
61+
}

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java‎

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import org.apache.hc.core5.io.CloseMode;
7373
import org.apache.hc.core5.pool.ConnPoolControl;
7474
import org.apache.hc.core5.pool.DefaultDisposalCallback;
75+
import org.apache.hc.core5.pool.DisposalCallback;
7576
import org.apache.hc.core5.pool.LaxConnPool;
7677
import org.apache.hc.core5.pool.ManagedConnPool;
7778
import org.apache.hc.core5.pool.PoolConcurrencyPolicy;
@@ -107,10 +108,13 @@
107108
*/
108109
@Contract(threading = ThreadingBehavior.SAFE_CONDITIONAL)
109110
public class PoolingHttpClientConnectionManager
110-
implements HttpClientConnectionManager, ConnPoolControl<HttpRoute> {
111+
implements HttpClientConnectionManager, ConnPoolControl<HttpRoute> {
111112

112113
private static final Logger LOG = LoggerFactory.getLogger(PoolingHttpClientConnectionManager.class);
113114

115+
private final DisposalCallback<ManagedHttpClientConnection> defaultDisposal;
116+
private final OffLockDisposalCallback<ManagedHttpClientConnection> offLockDisposer;
117+
114118
public static final int DEFAULT_MAX_TOTAL_CONNECTIONS = 25;
115119
public static final int DEFAULT_MAX_CONNECTIONS_PER_ROUTE = 5;
116120

@@ -216,16 +220,34 @@ public PoolingHttpClientConnectionManager(
216220
final PoolReusePolicy poolReusePolicy,
217221
final TimeValue timeToLive,
218222
final HttpConnectionFactory<ManagedHttpClientConnection> connFactory) {
223+
this(httpClientConnectionOperator,poolConcurrencyPolicy,poolReusePolicy,timeToLive,connFactory,false);
224+
225+
}
226+
227+
@Internal
228+
public PoolingHttpClientConnectionManager(
229+
final HttpClientConnectionOperator httpClientConnectionOperator,
230+
final PoolConcurrencyPolicy poolConcurrencyPolicy,
231+
final PoolReusePolicy poolReusePolicy,
232+
final TimeValue timeToLive,
233+
final HttpConnectionFactory<ManagedHttpClientConnection> connFactory,
234+
final boolean offLockDisposalEnabled) {
219235
super();
220236
this.connectionOperator = Args.notNull(httpClientConnectionOperator, "Connection operator");
237+
238+
this.defaultDisposal = new DefaultDisposalCallback<>();
239+
this.offLockDisposer = offLockDisposalEnabled ? new OffLockDisposalCallback<>(this.defaultDisposal) : null;
240+
final DisposalCallback<ManagedHttpClientConnection> callbackForPool = offLockDisposalEnabled ? this.offLockDisposer : this.defaultDisposal;
241+
242+
221243
switch (poolConcurrencyPolicy != null ? poolConcurrencyPolicy : PoolConcurrencyPolicy.STRICT) {
222244
case STRICT:
223245
this.pool = new StrictConnPool<HttpRoute, ManagedHttpClientConnection>(
224246
DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
225247
DEFAULT_MAX_TOTAL_CONNECTIONS,
226248
timeToLive,
227249
poolReusePolicy,
228-
new DefaultDisposalCallback<>(),
250+
callbackForPool,
229251
null) {
230252

231253
@Override
@@ -240,6 +262,7 @@ public void closeExpired() {
240262
DEFAULT_MAX_CONNECTIONS_PER_ROUTE,
241263
timeToLive,
242264
poolReusePolicy,
265+
callbackForPool,
243266
null) {
244267

245268
@Override
@@ -266,6 +289,8 @@ protected PoolingHttpClientConnectionManager(
266289
this.pool = Args.notNull(pool, "Connection pool");
267290
this.connFactory = connFactory != null ? connFactory : ManagedHttpClientConnectionFactory.INSTANCE;
268291
this.closed = new AtomicBoolean(false);
292+
this.defaultDisposal = null;
293+
this.offLockDisposer = null;
269294
}
270295

271296
@Override
@@ -280,6 +305,7 @@ public void close(final CloseMode closeMode) {
280305
LOG.debug("Shutdown connection pool {}", closeMode);
281306
}
282307
this.pool.close(closeMode);
308+
drainDisposals();
283309
LOG.debug("Connection pool shut down");
284310
}
285311
}
@@ -386,6 +412,10 @@ public ConnectionEndpoint get(
386412
}
387413
}
388414
}
415+
416+
// Single drain point under the lease lock.
417+
drainDisposals();
418+
389419
final ManagedHttpClientConnection conn = poolEntry.getConnection();
390420
if (conn != null) {
391421
conn.activate();
@@ -472,6 +502,7 @@ public void release(final ConnectionEndpoint endpoint, final Object state, final
472502
if (LOG.isDebugEnabled()) {
473503
LOG.debug("{} connection released {}", ConnPoolSupport.getId(endpoint), ConnPoolSupport.formatStats(entry.getRoute(), entry.getState(), pool));
474504
}
505+
drainDisposals();
475506
}
476507
}
477508

@@ -541,6 +572,7 @@ public void closeIdle(final TimeValue idleTime) {
541572
return;
542573
}
543574
this.pool.closeIdle(idleTime);
575+
drainDisposals();
544576
}
545577

546578
@Override
@@ -550,6 +582,7 @@ public void closeExpired() {
550582
}
551583
LOG.debug("Closing expired connections");
552584
this.pool.closeExpired();
585+
drainDisposals();
553586
}
554587

555588
@Override
@@ -825,16 +858,17 @@ public HttpConnection get() {
825858
}
826859

827860
/**
828-
* Method that can be called to determine whether the connection manager has been shut down and
829-
* is closed or not.
861+
* Returns whether this connection manager has been shut down.
830862
*
831-
* @return {@code true} if the connection manager has been shut down and is closed, otherwise
832-
* return {@code false}.
833863
* @since 5.4
834864
*/
835865
public boolean isClosed() {
836866
return this.closed.get();
837867
}
838868

839-
869+
private void drainDisposals() {
870+
if (offLockDisposer != null) {
871+
offLockDisposer.drain();
872+
}
873+
}
840874
}

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManagerBuilder.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
3939
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
4040
import org.apache.hc.client5.http.ssl.TlsSocketStrategy;
41+
import org.apache.hc.core5.annotation.Experimental;
4142
import org.apache.hc.core5.annotation.Internal;
4243
import org.apache.hc.core5.function.Resolver;
4344
import org.apache.hc.core5.http.HttpHost;
@@ -92,6 +93,8 @@ public class PoolingHttpClientConnectionManagerBuilder {
9293
private int maxConnTotal;
9394
private int maxConnPerRoute;
9495

96+
private boolean offLockDisposalEnabled;
97+
9598
public static PoolingHttpClientConnectionManagerBuilder create() {
9699
return new PoolingHttpClientConnectionManagerBuilder();
97100
}
@@ -304,6 +307,16 @@ public final PoolingHttpClientConnectionManagerBuilder useSystemProperties() {
304307
return this;
305308
}
306309

310+
/**
311+
* Enable/disable off-lock disposal.
312+
* @since 5.6
313+
*/
314+
@Experimental
315+
public final PoolingHttpClientConnectionManagerBuilder setOffLockDisposalEnabled(final boolean enabled) {
316+
this.offLockDisposalEnabled = enabled;
317+
return this;
318+
}
319+
307320
@Internal
308321
protected HttpClientConnectionOperator createConnectionOperator(
309322
final SchemePortResolver schemePortResolver,
@@ -332,7 +345,8 @@ public PoolingHttpClientConnectionManager build() {
332345
poolConcurrencyPolicy,
333346
poolReusePolicy,
334347
null,
335-
connectionFactory);
348+
connectionFactory,
349+
offLockDisposalEnabled);
336350
poolingmgr.setSocketConfigResolver(socketConfigResolver);
337351
poolingmgr.setConnectionConfigResolver(connectionConfigResolver);
338352
poolingmgr.setTlsConfigResolver(tlsConfigResolver);

‎httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/NoopOperator.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* ====================================================================
3+
* Licensed to the Apache Software Foundation (ASF) under one
4+
* or more contributor license agreements. See the NOTICE file
5+
* distributed with this work for additional information
6+
* regarding copyright ownership. The ASF licenses this file
7+
* to you under the Apache License, Version 2.0 (the
8+
* "License"); you may not use this file except in compliance
9+
* with the License. You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing,
14+
* software distributed under the License is distributed on an
15+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
* KIND, either express or implied. See the License for the
17+
* specific language governing permissions and limitations
18+
* under the License.
19+
* ====================================================================
20+
*
21+
* This software consists of voluntary contributions made by many
22+
* individuals on behalf of the Apache Software Foundation. For more
23+
* information on the Apache Software Foundation, please see
24+
* <http://www.apache.org/>.
25+
*
26+
*/
27+
package org.apache.hc.client5.http.impl.io;
28+
29+
import java.io.IOException;
30+
import java.net.InetSocketAddress;
31+
32+
import org.apache.hc.client5.http.io.HttpClientConnectionOperator;
33+
import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
34+
import org.apache.hc.core5.http.HttpHost;
35+
import org.apache.hc.core5.http.io.SocketConfig;
36+
import org.apache.hc.core5.http.protocol.HttpContext;
37+
import org.apache.hc.core5.util.TimeValue;
38+
39+
public final class NoopOperator implements HttpClientConnectionOperator {
40+
41+
@Override
42+
public void connect(
43+
final ManagedHttpClientConnection conn,
44+
final HttpHost host,
45+
final InetSocketAddress localAddress,
46+
final TimeValue connectTimeout,
47+
final SocketConfig socketConfig,
48+
final HttpContext context) throws IOException {
49+
// no-op for tests
50+
}
51+
52+
@Override
53+
public void upgrade(
54+
final ManagedHttpClientConnection conn,
55+
final HttpHost host,
56+
final HttpContext context) throws IOException {
57+
// no-op for tests
58+
}
59+
}

0 commit comments

Comments
 (0)