Fix NPE in GraphTransaction.doClose() on concurrent graph close (#4899)

[batrived]

Fixes #4899.

Problem

There is a race between JanusGraphBlueprintsGraph.close() and GraphTransaction.doClose(). close() sets the shared txs ThreadLocal field to null, but doClose() called txs.remove() without a null check. If a graph is closed on one thread while another thread is finishing a commit()/rollback(), the committing thread throws an NPE in its finally cleanup block — after the storage commit has already been attempted, masking the real outcome.

This commonly surfaces during application shutdown / container termination, where in-flight Gremlin requests overlap with graph close. Note that getAutoStartTx() and GraphTransaction.isOpen() already guard against txs == null, but doClose() did not — the null-handling was inconsistent.

Fix

Snapshot the shared txs reference to a local and null-check it before calling remove():

protected void doClose() {
    super.doClose();
    transactionListeners.remove();
    ThreadLocal<JanusGraphBlueprintsTransaction> localTxs = txs;
    if (localTxs != null) localTxs.remove();
}

This preserves the txs = null behavior from #1653 (which intentionally drops the strong reference to allow ThreadLocal cleanup, see #1341) while making cleanup null-safe.

Testing

Added a regression test ThreadLocalTxLeakTest#doCloseMustBeNullSafeWhenTxsClearedConcurrently that simulates a concurrent close() nulling the field and asserts doClose() does not throw. Verified it fails with an NPE without the fix and passes with it. All tests in the class pass.

---

For all changes:

• Is there an issue associated with this PR? Is it referenced in the commit message?
• Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
• Has your PR been rebased against the latest commit within the target branch (typically master)?
• Is your initial contribution a single, squashed commit?

For code changes:

• Have you written and/or updated unit tests to verify your changes?
• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0? (N/A — no new dependencies)
• If applicable, have you updated the LICENSE.txt file? (N/A)
• If applicable, have you updated the NOTICE.txt file? (N/A)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: batrived / name: Balmukund Trivedi (a5fc7fb)

[Copilot]

Pull request overview

This PR addresses a concurrency race in JanusGraph’s thread-bound transaction cleanup where JanusGraphBlueprintsGraph.close() can null the shared txs ThreadLocal while another thread is executing GraphTransaction.doClose(), previously causing an NPE during commit/rollback cleanup and masking the real transaction outcome.

Changes:

• Make GraphTransaction.doClose() null-safe by snapshotting txs to a local variable and checking for null before calling remove().
• Add a regression test that simulates txs being nulled concurrently and asserts doClose() does not throw.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
janusgraph-core/src/main/java/org/janusgraph/graphdb/tinkerpop/JanusGraphBlueprintsGraph.java	Makes transaction cleanup tolerant of txs being nulled during concurrent graph close.
janusgraph-test/src/test/java/org/janusgraph/core/ThreadLocalTxLeakTest.java	Adds regression coverage to ensure doClose() remains null-safe when txs is cleared.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[porunov]

LGTM. Thank you @batrived !

[batrived]

@porunov thank you for your time and reviewing it. Since I don't have merge permissions, could you please merge it when you get a chance? Thanks!

[porunov]

@porunov thank you for your time and reviewing it. Since I don't have merge permissions, could you please merge it when you get a chance? Thanks!

Based on the standard RTC rules we either need a second approval or a week for lazy consensus: https://docs.janusgraph.org/development/#review-then-commit-rtc