Skip to content

HTTPCLIENT-2416 - Fix pool entry leak on late lease completion#809

Open
arturobernalg wants to merge 1 commit intoapache:masterfrom
arturobernalg:HTTPCLIENT-2416
Open

HTTPCLIENT-2416 - Fix pool entry leak on late lease completion#809
arturobernalg wants to merge 1 commit intoapache:masterfrom
arturobernalg:HTTPCLIENT-2416

Conversation

@arturobernalg
Copy link
Member

This patch fixes a race in PoolingHttpClientConnectionManager#lease().

When LeaseRequest#get(...) exits with TimeoutException or InterruptedException, the current code calls leaseFuture.cancel(true). If the underlying lease completes just before cancellation is observed, cancel(true) returns false and the PoolEntry can remain leased, because no ConnectionEndpoint was returned to the caller and nothing releases it.

The fix handles that case in the exceptional path: if cancel(true) returns false, the code attempts a zero-timeout get() on the lease future and, if a late-completed PoolEntry is obtained, releases it immediately back to the pool.

@arturobernalg arturobernalg requested a review from ok2c March 5, 2026 15:25
@arturobernalg arturobernalg force-pushed the HTTPCLIENT-2416 branch 2 times, most recently from f8fc570 to 1865484 Compare March 5, 2026 15:52
} catch (final TimeoutException | InterruptedException ex) {
if (!leaseFuture.cancel(true)) {
try {
final PoolEntry<HttpRoute, ManagedHttpClientConnection> latePoolEntry = leaseFuture.get(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturobernalg If the method fails once the proposed fix to call another time? That does not look right. There has to be a better fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ok2c I dropped the second get() and reworked it to use the pool.lease(..., FutureCallback) to capture late completions and release the PoolEntry on timeout/interrupt/cancel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants