benjaminp commented on PR #809:
URL:
https://github.com/apache/httpcomponents-client/pull/809#issuecomment-4007343485
Thanks, this does seem to resolve my examples raised on the ticket.
I do have a suggested fix of my own, which is a bit less code:
```
diff --git
a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalExecRuntime.java
b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalExecRuntime.java
index 7813bc171..dbc7d17b1 100644
---
a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalExecRuntime.java
+++
b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/InternalExecRuntime.java
@@ -28,6 +28,7 @@
package org.apache.hc.client5.http.impl.classic;
import java.io.IOException;
+import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;
@@ -118,10 +119,10 @@ public void acquireEndpoint(
log.debug("{} acquired endpoint {}", id,
ConnPoolSupport.getId(connectionEndpoint));
}
} catch (final TimeoutException ex) {
- connRequest.cancel();
+ cancelLeaklessly(connRequest);
throw new
ConnectionRequestTimeoutException(ex.getMessage());
} catch (final InterruptedException interrupted) {
- connRequest.cancel();
+ cancelLeaklessly(connRequest);
Thread.currentThread().interrupt();
throw new RequestFailedException("Request aborted",
interrupted);
} catch (final ExecutionException ex) {
@@ -137,6 +138,18 @@ public void acquireEndpoint(
}
}
+ private void cancelLeaklessly(final LeaseRequest request) {
+ if (!request.cancel()) {
+ try {
+ discardEndpoint(request.get(Timeout.INFINITE));
+ } catch (final InterruptedException | TimeoutException e) {
+ throw new IllegalStateException(e);
+ } catch (final ExecutionException | CancellationException e) {
+ // ignore
+ }
+ }
+ }
+
ConnectionEndpoint ensureValid() {
final ConnectionEndpoint endpoint = endpointRef.get();
if (endpoint == null) {
```
The idea is the check if cancelation on `InterruptedException` or
`TimeoutException` actually mutates future. If it did not, it means the future
was already completed and may have a connection that must be released.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
