reta commented on code in PR #3673:
URL: https://github.com/apache/cxf/pull/3673#discussion_r4518745356
##########
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:
Sorry missed that before. Intuitively, `cl.release()` may close the client
(invalidating `cl` instance), but it should never happen I believe
(deferredClientRefs should have a least one reference, and
`CLIENTS_CACHE.computeIfAbsent` should add at least one more), could we just do
`cl.isClosed()` here?
```
if (deferredClientRefs.remove(cl)) {
cl.release();
if (cl.isClosed()) {
throw new IllegalStateException("The client has been closed
already");
}
}
```
WDYT?
--
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]
