eric6iese commented on code in PR #2571:
URL: https://github.com/apache/cxf/pull/2571#discussion_r2299742778
##########
rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpClientHTTPConduit.java:
##########
@@ -125,10 +125,18 @@ private static final class RefCount<T extends HttpClient>
{
this.policy = policy;
this.clientParameters = clientParameters;
this.finalizer = finalizer;
+ this.count = new AtomicLong(1);
}
RefCount<T> acquire() {
Review Comment:
If am not mistaken, there should be simpler way to implement this. If you
change acquire to boolean, then there is no need for an exception or the
isClosed-Method.
```java
boolean acquire() {
while (true) {
final long c = count.get();
if (c == 0L) {
return false; // skip closed but not yet removed clients
} else if (count.compareAndSet(c, c + 1)) {
return true;
}
}
}
```
The section below than changes to:
```java
for (final RefCount<HttpClient> p: clients) {
if (cpc.equals(p.policy(), policy) &&
p.clientParameters().equals(clientParameters) && p.acquire()) {
return p;
}
}
```
To be honest, I am still not sure if this is safe on all paths. If I would
write this routine, then I would use a simple long in the refcounter and add a
field for the lock above to use for release as well to avoid any possible race
condition between these two methods. But that might come at its own costs and
complexities.
##########
rt/transports/http/src/main/java/org/apache/cxf/transport/http/HttpClientHTTPConduit.java:
##########
@@ -195,23 +207,29 @@ RefCount<HttpClient> computeIfAbsent(final boolean
shareHttpClient, final HTTPCl
// Do not share if it is not allowed for the conduit or cache
capacity is exceeded
if (!shareHttpClient || clients.size() >= MAX_SIZE) {
Review Comment:
While not part of the PR, I wanted to note that Synchronization needs to
happen along all read/write-paths of an collection which is not threadsafe,
like the clients-ArrayList.
Even a simple operation like clients.size() is not safe to be invoked while
another thread manipulates the list. I would recommend to move these lines in
the locking-section below.
--
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]
