Skip to content

SOLR-18472 TikaServerExtractionBackend can hand out a reference to a stopped HttpClient - #4930

Open
janhoy wants to merge 4 commits into
apache:mainfrom
janhoy:SOLR-18472-tikaserver-httpclient-refcount
Open

janhoy wants to merge 4 commits into
apache:mainfrom
janhoy:SOLR-18472-tikaserver-httpclient-refcount

Conversation

@janhoy

@janhoy janhoy commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/SOLR-18472

The fix is doing both incref and decref synchronized on the lock.

Code written by Claude Code Opus. Reviewed by Copilot. Fixed by Claude Fable. Reviewed by a Human™

The unlocked fast path in initializeHttpClient() could hand out a reference
that a concurrent close() dropped to zero refs, leaving a new backend with a
stopped HttpClient. Acquire (incref) and release (decref) now both happen
under INIT_LOCK.
…s handle

Nulling acquiredResourcesRef left request threads reading a field that could
become null mid-request, and raced with the unsynchronized read in
callTikaServer(). The handle is now final and double-close is guarded by a
CAS instead.

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The concurrency fix needs regression coverage for simultaneous acquisition and closure.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Fixes a race where Tika extraction backends could acquire a stopped shared HTTP client.

Changes:

  • Synchronizes shared resource acquisition and release.
  • Makes backend closure idempotent.
  • Documents the fix in the changelog.
File Description
TikaServerExtractionBackend.java Safely manages shared HTTP-client references.
SOLR-18472-tikaserver-httpclient-refcount.yml Adds the changelog entry.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…race

Race the last backend's close() against construction of a new backend
and verify the new backend still completes a request, and check that
calling close() twice does not tear down resources still in use.
@github-actions github-actions Bot added the tests label Oct 7, 2026
@janhoy
janhoy marked this pull request as ready for review October 7, 2026 20:33
@janhoy
janhoy requested a review from epugh October 7, 2026 20:34
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