Description
There is a race between JanusGraphBlueprintsGraph.close() and GraphTransaction.doClose(). close() sets the shared txs ThreadLocal field to null, but doClose() calls 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.
Relevant code (janusgraph-core, JanusGraphBlueprintsGraph)
public synchronized void close() {
txs.remove();
txs = null; // shared field nulled
}
protected void doClose() {
super.doClose();
transactionListeners.remove();
txs.remove(); // NPE if close() already ran on another thread
}
Note that getAutoStartTx() and GraphTransaction.isOpen() already guard against txs == null, but doClose() does not — the null-handling is inconsistent.
Impact
The NPE happens in a finally block after super.commit(), so the transaction may have actually committed. Callers see an NPE instead of the true commit result, making the outcome ambiguous.
Suggested fix
Null-check the txs reference in doClose() (snapshot to a local first). 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:
protected void doClose() {
super.doClose();
transactionListeners.remove();
ThreadLocal<JanusGraphBlueprintsTransaction> localTxs = txs;
if (localTxs != null) localTxs.remove();
}
Related: #1341, #1653.
Version
Observed against the JanusGraphBlueprintsGraph cleanup logic as currently on master.
Description
There is a race between
JanusGraphBlueprintsGraph.close()andGraphTransaction.doClose().close()sets the sharedtxsThreadLocal field tonull, butdoClose()callstxs.remove()without a null check. If a graph is closed on one thread while another thread is finishing acommit()/rollback(), the committing thread throws an NPE in itsfinallycleanup 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.
Relevant code (
janusgraph-core,JanusGraphBlueprintsGraph)Note that
getAutoStartTx()andGraphTransaction.isOpen()already guard againsttxs == null, butdoClose()does not — the null-handling is inconsistent.Impact
The NPE happens in a
finallyblock aftersuper.commit(), so the transaction may have actually committed. Callers see an NPE instead of the true commit result, making the outcome ambiguous.Suggested fix
Null-check the
txsreference indoClose()(snapshot to a local first). This preserves thetxs = nullbehavior from #1653 (which intentionally drops the strong reference to allow ThreadLocal cleanup, see #1341) while making cleanup null-safe:Related: #1341, #1653.
Version
Observed against the
JanusGraphBlueprintsGraphcleanup logic as currently onmaster.