Skip to content

SOLR-17707: release the content-writing thread when an async request fails#4639

Merged
dsmiley merged 2 commits into
apache:mainfrom
serhiy-bzhezytskyy:SOLR-17707-httpjdk-async-thread-leak
Jul 16, 2026
Merged

SOLR-17707: release the content-writing thread when an async request fails#4639
dsmiley merged 2 commits into
apache:mainfrom
serhiy-bzhezytskyy:SOLR-17707-httpjdk-async-thread-leak

Conversation

@serhiy-bzhezytskyy

Copy link
Copy Markdown
Contributor

Picking up the POC Jan left on SOLR-17707.

HttpJdkSolrClient's async paths (requestAsync, requestInputStreamAsync) never cleaned up the content-writing task. When a connection fails while the request body is still being written, that thread stays blocked forever in PipedInputStream.awaitSpace(). Because the writer runs on the client's own executor, enough of these exhaust the pool and the client stops working — the thread dump in the issue shows exactly this. The synchronous requestWithBaseUrl already cancels the writer in its finally; the async paths did not.

Both async paths now release the writer via whenComplete.

One thing worth calling out for review: cancelling the Future alone does not fix it. In a repro I found cancel(true) returns true but the interrupt does not reliably reach the thread blocked in the pipe (it stays TIMED_WAITING, interrupted=false). Closing the pipe's sink is what deterministically unblocks the writer (IOException: Pipe closed). The fix does both — close to release a stuck writer, cancel to stop one that hasn't started.

Test: testStuckContentWritingThreadIsReleasedOnFailure sends a body larger than the pipe/socket buffers to a server that accepts but never reads, then drops the connection, and asserts no writer thread is left blocked in awaitSpace(). It fails without the fix and passes with it. This is the reliable reproduction that was hard to pin down earlier — the key was a body big enough to actually block the writer before the failure.

…fails

HttpJdkSolrClient's async paths never cleaned up the content-writing task, so a
connection failure while the body was being written left that thread blocked
forever in PipedInputStream.awaitSpace(). Since the writer shares the client's
executor, enough such failures exhaust the pool. The synchronous path already
cancels the writer in its finally block; the async paths did not.

Both async paths now release the writer on completion. Closing the pipe's sink
is what unblocks a writer already stuck in awaitSpace(); cancelling the future
alone does not, since the interrupt does not reliably reach the blocked thread.

@dsmiley dsmiley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Very valuable fix.

Import CountDownLatch instead of fully-qualifying it, and wait for the request
to settle via handle().get() rather than a catch that errorprone flags as
MissingFail.

@dsmiley dsmiley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm so glad to see a fix here!

@dsmiley dsmiley added this to the 9.x milestone Jul 16, 2026
@dsmiley dsmiley merged commit aa9e3f1 into apache:main Jul 16, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants