[
https://issues.apache.org/jira/browse/HTTPCLIENT-2432?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18115043#comment-18115043
]
Alekseev Victor commented on HTTPCLIENT-2432:
---------------------------------------------
This issue was observed in combination with SAP instance. The idea for the fix:
Guarantee (B) runs regardless of (A):
{code:java}
private void cleanup() throws IOException {
if (this.execRuntime != null) {
try {
if (this.execRuntime.isEndpointConnected()) {
this.execRuntime.disconnectEndpoint();
}
} finally {
this.execRuntime.discardEndpoint();
}
}
}{code}
This is safe:
- discardEndpoint() is idempotent — its first line is
endpointRef.getAndSet(null), so a redundant call after a normal
releaseEndpoint() / prior discardEndpoint() is a no-op.
- The pool release inside discardEndpoint() is already in a finally, so
endpoint.close(CloseMode.IMMEDIATE) throwing on a dead socket does not prevent
manager.release(...).
- On the healthy path the observable behavior is unchanged: the endpoint was
already released via releaseConnection() before cleanup() ran, endpointRef is
null, and the added finally no-ops.
> Connection permanently marked leased when ResponseEntityProxy.cleanup() fails
> on a dead socket
> ----------------------------------------------------------------------------------------------
>
> Key: HTTPCLIENT-2432
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-2432
> Project: HttpComponents HttpClient
> Issue Type: Bug
> Components: HttpClient (classic)
> Affects Versions: 5.6.4
> Environment: - org.apache.httpcomponents.client5:httpclient5:5.6.4
> - Behavior present since the cleanup() shape was introduced; unchanged in
> HEAD at time of filing (please verify against 5.x branch).
> Reporter: Alekseev Victor
> Priority: Major
>
> ResponseEntityProxy#cleanup() invokes two sequential calls on ExecRuntime
> without a finally guard:
> private void cleanup() throws IOException {
> if (this.execRuntime != null) {
> if (this.execRuntime.isEndpointConnected()) {
> this.execRuntime.disconnectEndpoint(); // (A)
> }
> this.execRuntime.discardEndpoint(); // (B)
> }
> }
> (A) InternalExecRuntime.disconnectEndpoint() calls endpoint.close() and can
> throw IOException when the underlying socket is in a broken state. It does
> not touch endpointRef.
> (B) InternalExecRuntime.discardEndpoint() is the only path that returns the
> lease to the pool — it does endpointRef.getAndSet(null) and, inside a
> finally, manager.release(endpoint, null, TimeValue.ZERO_MILLISECONDS).
> When (A) throws, (B) is skipped. endpointRef stays populated,
> manager.release(...) is never invoked, and PoolingHttpClientConnectionManager
> continues to count the connection as leased for the entire lifetime of the
> client.
> Steps to reproduce
> 1. Configure a classic CloseableHttpClient with a PoolingHttpClie=N,
> maxPerRoute=N).
> 2. Perform a request against an endpoint that returns a streaming body and
> whose transport (proxy, TLS tunnel, load balancer) drops the connection with
> RST
> mid-body — before EOF is reached.
> 3. Observe PoolTracker/getTotalStats().getLeased() after each such failure.
> 4. Repeat under a retry stack (application-level, RetryTemplate,
> A minimal deterministic reproducer: a stub server that closes theg status
> line + a partial chunk; wrap the client in a loop withretries.
> Expected
> After the response entity is closed (or cleanup() runs from any of close() /
> writeTo() / eofDetected() / streamClosed() / streamAbort()), the leased
> counter must return to its pre-request value regardless of whether the gr.
> Actual
> leased monotonically increases by one every time disconnectEndpoihes
> maxTotal, all subsequent lease() calls block untilconnectionRequestTimeout
> and then fail with ConnectionRequestTimeoutException. The client is
> effectively dead for the remainder of its lifetime.
> Impact
> - Silent pool exhaustion under any transient network fault that surfaces at
> socket close.
> - Amplified by any retry layer above the client — one bad tunnel in a single
> logical operation.
> - No warning is logged; the leaked entries look like healthy leased
> connections in JMX/pool stats.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
