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

janusgraph-core/src/main/java/org/janusgraph/graphdb/tinkerpop/JanusGraphBlueprintsGraph.java

@@ -344,9 +344,13 @@ public boolean isOpen() {
 
         @Override
         protected void doClose() {
+            // Snapshot the shared txs reference first: close() may null it out concurrently on another
+            // thread. Capturing it before the calls below keeps the race window minimal so this thread
+            // still removes its own ThreadLocal entry rather than relying on delayed GC-based cleanup.
+            ThreadLocal<JanusGraphBlueprintsTransaction> localTxs = txs;
             super.doClose();
             transactionListeners.remove();
-            txs.remove();
+            if (localTxs != null) localTxs.remove();
         }
 
         @Override

janusgraph-test/src/test/java/org/janusgraph/core/ThreadLocalTxLeakTest.java

@@ -108,6 +108,56 @@
         assertThreadLocalClosed();
     }
 
+    /**
+     * Reproduces https://github.com/JanusGraph/janusgraph/issues/4899: when the graph is closed on another
+     * thread (nulling the shared {@code txs} field) while a transaction is being closed, {@code doClose()}
+     * used to throw a {@link NullPointerException} in its cleanup block, masking the real commit/rollback
+     * outcome. After the fix, {@code doClose()} snapshots the {@code txs} reference and tolerates a null,
+     * so the cleanup completes without throwing.
+     */
+    @Test
+    void doCloseMustBeNullSafeWhenTxsClearedConcurrently() throws Exception {
+        CompletableFuture.runAsync(() -> {
+            // Simulate a concurrent close() that has already nulled the shared txs field before this
+            // thread reaches the doClose() cleanup that runs in commit()/rollback()'s finally block.
+            ThreadLocal<?> original = getThreadLocalTxs(graph);
+            setThreadLocalTxs(graph, null);
+            try {
+                Assertions.assertDoesNotThrow(() -> invokeDoClose(graph),
+                    "doClose() must not throw when txs has been nulled concurrently");
+            } finally {
+                // Restore the field so the @AfterEach close() does not trip over the artificially nulled txs.
+                setThreadLocalTxs(graph, original);
+            }
+        }, executorService).get();
+    }
+
+    private void invokeDoClose(JanusGraph graph) {
+        try {
+            Field container = JanusGraphBlueprintsGraph.class.getDeclaredField("tinkerpopTxContainer");
+            container.setAccessible(true);
+            Object graphTransaction = container.get(graph);
+            java.lang.reflect.Method doClose = graphTransaction.getClass().getDeclaredMethod("doClose");
+            doClose.setAccessible(true);
+            doClose.invoke(graphTransaction);
+        } catch (ReflectiveOperationException e) {
+            if (e.getCause() instanceof RuntimeException) {
+                throw (RuntimeException) e.getCause();
+            }
+            throw new JanusGraphException(e);
+        }
+    }
+
+    private void setThreadLocalTxs(JanusGraph graph, ThreadLocal<?> value) {
+        try {
+            Field txs = JanusGraphBlueprintsGraph.class.getDeclaredField("txs");
+            txs.setAccessible(true);
+            txs.set(graph, value);
+        } catch (IllegalAccessException | NoSuchFieldException e) {
+            throw new JanusGraphException(e);
+        }
+    }
+
     private void assertThreadLocalClosed() {
         Assertions.assertDoesNotThrow(() -> {
             CompletableFuture.runAsync(