Skip to content

SOLR-18578: Close AutoCloseable-only objects in ObjectReleaseTracker.tryClose() - #5213

Open
nick-boss-tech wants to merge 1 commit into
apache:mainfrom
nick-boss-tech:phone-agent-2-objectrelease-autocloseable-candidate
Open

nick-boss-tech wants to merge 1 commit into
apache:mainfrom
nick-boss-tech:phone-agent-2-objectrelease-autocloseable-candidate

Conversation

@nick-boss-tech

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

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

What happens today

ObjectReleaseTracker.tryClose(), the test framework's failure cleanup, never closes tracked objects that implement only AutoCloseable.

tryClose() handles two kinds of tracked objects: Closeable and ExecutorService. SolrTestCase calls it after a test class fails, so anything it skips stays open during failure recovery. Three tracked types implement only AutoCloseable: SolrMetricsContext, which tracks itself and whose close() closes its registered resources; ZkCollectionTerms; and ZkShardTerms. A leaked metrics context therefore survives the cleanup pass with its registered resources open. SOLR-17857 added the tracker registration for SolrMetricsContext; the two-branch shape of tryClose() dates to SOLR-15550 and was not widened then.

Change

tryClose() gains a final AutoCloseable branch; the Closeable and ExecutorService branches are unchanged.

The branch order preserves the executor special case, which shuts down through ExecutorUtil.shutdownAndAwaitTermination, and a close failure is logged while cleanup continues, matching the existing branches. The new branch is six lines: ObjectReleaseTracker.java at the head. Before the branch was added, every tracked AutoCloseable-only type was checked for close safety: SolrMetricsContext.close() releases its own tracker entry and closes registered resources quietly; ZkCollectionTerms.close() closes the contained shard terms under its lock; ZkShardTerms.close() is repeat-safe, which matters because a collection close can close a shard term the tracker iteration later reaches on its own.

TestObjectReleaseTracker gains five tests: a minimal AutoCloseable-only tracked object is closed exactly once and untracked; cleanup continues after one close() throws; a real SolrMetricsContext with a registered resource is closed; a tracked parent/child pair ends in the intended state when both are in the tracker; the executor branch still handles an ExecutorService. Each new test clears the global tracker in a finally block.

Proof

At head 35faf539e94, with seed 3B1F3E98A7C2D144: TestObjectReleaseTracker runs 10 tests with 0 failures, 0 errors, 0 skipped (the class had 5 tests before this change). With only ObjectReleaseTracker.java restored to its base version and this head's tests kept, four of the five new tests fail, each because the tracked object's close counter stays at 0: testTryCloseAutoCloseable, testTryCloseContinuesAfterAutoCloseableFailure, testTryCloseSolrMetricsContextClosesRegisteredResources and testTryCloseNestedAutoCloseables. The fifth new test passes on both versions, as it should: it pins the pre-existing executor branch. Tidy passes on the changed files. :solr:core:check -x test and :solr:solrj:check -x test pass with -Pvalidation.errorprone=true. No full module test suite was run.

Limits

  • The change is limited to the test-failure fallback. Normal production shutdown is untouched, and no production class other than ObjectReleaseTracker changes.
  • The fallback is a plausible amplifier of leak noise after test failures; nothing here claims it caused any specific past CI failure.

Checklist

  • I have reviewed the contributing docs.
  • I have created a Jira issue and added the issue ID to my pull request title. SOLR-18578
  • (Optional) I have added a changelog entry to the changelog fragment directory.

🤖 AI footer 🤖 This pull request description was written by an AI assistant on behalf of Nick Shanin, who directed the work and takes responsibility for the contribution.

@nick-boss-tech
nick-boss-tech marked this pull request as ready for review October 23, 2026 19: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.

1 participant