ffang commented on code in PR #3673:
URL: https://github.com/apache/cxf/pull/3673#discussion_r4519073562
##########
rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpClientHTTPConduit.java:
##########
@@ -473,7 +544,13 @@ protected void setupConnection(Message message, Address
address, HTTPClientPolic
if (cl == null) {
final boolean shareHttpClient =
MessageUtils.getContextualBoolean(message,
SHARE_HTTPCLIENT_CONDUIT, true);
- cl = CLIENTS_CACHE.computeIfAbsent(shareHttpClient,
csPolicy, clientParameters, () -> cb.build());
+ cl = CLIENTS_CACHE.computeIfAbsent(shareHttpClient,
clientPolicy, clientParametersCopy,
+ () -> cb.build());
+ // the cache may hand back a shared client this conduit
has stopped using, when it
+ // switches back to its settings or its HTTPS address
changes. Keep a single reference
+ if (deferredClientRefs.remove(cl)) {
+ cl.release();
Review Comment:
Hi @reta,
Right, it shouldn't happen: remove(cl) only succeeds when cl came back from
the shared cache, so the conduit holds both the deferred reference and the one
computeIfAbsent just acquired. acquire() refuses a closed client, and both
references are only released by this conduit under initializationLock, so the
release leaves at least one. I've added the isClosed() check as you suggested,
with a comment saying why it shouldn't happen, so a reference-counting bug
would fail clearly here.
Cheers
Freeman
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
