Merge branch '286-close-cursor-on-write-tx-exception' into dev

Author: greenrobot-team

CHANGELOG.md

@@ -4,7 +4,10 @@ Notable changes to the ObjectBox Java library.
 
 For more insights into what changed in the ObjectBox C++ core, [check the ObjectBox C changelog](https://github.com/objectbox/objectbox-c/blob/main/CHANGELOG.md).
 
-## 5.0.2 - in development
+## Next release
+
+- `BoxStore.runInTx` and `callInTx` close a write cursor even if the runnable or callable throws. This would previously
+  result in cursor not closed warnings when the cursor was closed by the finalizer daemon. 
 
 ## 5.0.1 - 2025-09-30
 

objectbox-java/src/main/java/io/objectbox/Box.java

@@ -23,7 +23,6 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.Callable;
 
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
@@ -89,6 +88,13 @@ Cursor<T> getReader() {
         return cursor;
     }
 
+    /**
+     * Returns if for the calling thread this has a Cursor, if any, for the currently active transaction.
+     */
+    boolean hasActiveTxCursorForCurrentThread() {
+        return activeTxCursor.get() != null;
+    }
+
     Cursor<T> getActiveTxCursor() {
         Transaction activeTx = store.activeTx.get();
         if (activeTx != null) {
@@ -166,20 +172,16 @@ public void closeThreadResources() {
         }
     }
 
-    void txCommitted(Transaction tx) {
-        // Thread local readers will be renewed on next get, so we do not need clean them up
-
-        Cursor<T> cursor = activeTxCursor.get();
-        if (cursor != null) {
-            activeTxCursor.remove();
-            cursor.close();
-        }
-    }
-
     /**
-     * Called by {@link BoxStore#callInReadTx(Callable)} - does not throw so caller does not need try/finally.
+     * If there is one, and it was created using the given {@code tx}, removes and closes the {@link #activeTxCursor}
+     * for the current thread.
+     * <p>
+     * This should be called before the active transaction is closed to clean up native resources.
+     * <p>
+     * Note: {@link #threadLocalReader} is either renewed by the next call to {@link #getReader()} or cleaned up by
+     * {@link #closeThreadResources()}.
      */
-    void readTxFinished(Transaction tx) {
+    void closeActiveTxCursorForCurrentThread(Transaction tx) {
         Cursor<T> cursor = activeTxCursor.get();
         if (cursor != null && cursor.getTx() == tx) {
             activeTxCursor.remove();

objectbox-java/src/main/java/io/objectbox/BoxStore.java

@@ -926,15 +926,20 @@ void txCommitted(Transaction tx, @Nullable int[] entityTypeIdsAffected) {
             }
         }
 
-        for (Box<?> box : boxes.values()) {
-            box.txCommitted(tx);
-        }
-
         if (entityTypeIdsAffected != null) {
             objectClassPublisher.publish(entityTypeIdsAffected);
         }
     }
 
+    /**
+     * For all boxes, calls {@link Box#closeActiveTxCursorForCurrentThread(Transaction)}.
+     */
+    void closeActiveTxCursorsForCurrentThread(Transaction tx) {
+        for (Box<?> box : boxes.values()) {
+            box.closeActiveTxCursorForCurrentThread(tx);
+        }
+    }
+
     /**
      * Returns a Box for the given type. Objects are put into (and get from) their individual Box.
      * <p>
@@ -977,6 +982,7 @@ public void runInTx(Runnable runnable) {
                 tx.commit();
             } finally {
                 activeTx.remove();
+                closeActiveTxCursorsForCurrentThread(tx);
                 tx.close();
             }
         } else {
@@ -1003,13 +1009,9 @@ public void runInReadTx(Runnable runnable) {
                 runnable.run();
             } finally {
                 activeTx.remove();
-
                 // TODO That's rather a quick fix, replace with a more general solution
                 // (that could maybe be a TX listener with abort callback?)
-                for (Box<?> box : boxes.values()) {
-                    box.readTxFinished(tx);
-                }
-
+                closeActiveTxCursorsForCurrentThread(tx);
                 tx.close();
             }
         } else {
@@ -1089,10 +1091,7 @@ public <T> T callInReadTx(Callable<T> callable) {
 
                 // TODO That's rather a quick fix, replace with a more general solution
                 // (that could maybe be a TX listener with abort callback?)
-                for (Box<?> box : boxes.values()) {
-                    box.readTxFinished(tx);
-                }
-
+                closeActiveTxCursorsForCurrentThread(tx);
                 tx.close();
             }
         } else {
@@ -1119,6 +1118,7 @@ public <R> R callInTx(Callable<R> callable) throws Exception {
                 return result;
             } finally {
                 activeTx.remove();
+                closeActiveTxCursorsForCurrentThread(tx);
                 tx.close();
             }
         } else {

tests/objectbox-java-test/src/test/java/io/objectbox/TransactionTest.java

@@ -30,6 +30,7 @@
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -422,12 +423,90 @@ public void testCallInReadTx() {
     }
 
     @Test
-    public void testRunInReadTxAndThenPut() {
+    public void testRunInTx_closesActiveTxCursor() {
         final Box<TestEntity> box = getTestEntityBox();
-        store.runInReadTx(box::count);
-        // Verify that box does not hang on to the read-only TX by doing a put
+        AtomicBoolean hasCursorInTx = new AtomicBoolean(false);
+        store.runInTx(() -> {
+            box.count(); // Call Box API that creates a reader/activeTxCursor
+            hasCursorInTx.set(box.hasActiveTxCursorForCurrentThread());
+        });
+        // Verify a cursor for the active tx was created
+        assertTrue(hasCursorInTx.get());
+        // Check it was released
+        assertFalse(box.hasActiveTxCursorForCurrentThread());
+
+        // Verify the same in case the runnable throws
+        try {
+            store.runInTx(() -> {
+                box.count();
+                throw new IllegalStateException("Throw in transaction");
+            });
+        } catch (IllegalStateException ignored) {}
+        assertFalse(box.hasActiveTxCursorForCurrentThread());
+    }
+
+    @Test
+    public void testCallInTx_closesActiveTxCursor() throws Exception {
+        final Box<TestEntity> box = getTestEntityBox();
+        Boolean hasCursorInTx = store.callInTx(() -> {
+            box.count(); // Call Box API that creates a reader/activeTxCursor
+            return box.hasActiveTxCursorForCurrentThread();
+        });
+        // Verify a cursor for the active tx was created
+        assertTrue(hasCursorInTx);
+        // Check it was released
+        assertFalse(box.hasActiveTxCursorForCurrentThread());
+
+        // Verify the same in case the callable throws
+        try {
+            store.callInTx(() -> {
+                box.count();
+                throw new IllegalStateException("Throw in transaction");
+            });
+        } catch (IllegalStateException ignored) {}
+        assertFalse(box.hasActiveTxCursorForCurrentThread());
+    }
+
+    @Test
+    public void testRunInReadTx_closesActiveTxCursor() {
+        final Box<TestEntity> box = getTestEntityBox();
+        store.runInReadTx(box::count); // Call Box API that creates a reader/activeTxCursor
+        // Verify that box does not hang on to the read-only TX: if it would, count() would re-use the cursor/tx from
+        // above and not see the put object.
+        putTestEntityAndExpectCount(1);
+        assertFalse(box.hasActiveTxCursorForCurrentThread());
+
+        // Verify the same in case the runnable throws
+        assertThrows(IllegalStateException.class, () -> store.runInReadTx(() -> {
+            box.count();
+            throw new IllegalStateException("Throw in transaction");
+        }));
+        putTestEntityAndExpectCount(2);
+        assertFalse(box.hasActiveTxCursorForCurrentThread());
+    }
+
+    @Test
+    public void testCallInReadTx_closesActiveTxCursor() {
+        final Box<TestEntity> box = getTestEntityBox();
+        store.callInReadTx(box::count); // Call Box API that creates a reader/activeTxCursor
+        // Verify that box does not hang on to the read-only TX: if it would, count() would re-use the cursor/tx from
+        // above and not see the put object.
+        putTestEntityAndExpectCount(1);
+        assertFalse(box.hasActiveTxCursorForCurrentThread());
+
+        // Verify the same in case the callable throws
+        assertThrows(IllegalStateException.class, () -> store.callInReadTx(() -> {
+            box.count();
+            throw new IllegalStateException("Throw in transaction");
+        }));
+        putTestEntityAndExpectCount(2);
+        assertFalse(box.hasActiveTxCursorForCurrentThread());
+    }
+
+    private void putTestEntityAndExpectCount(int expectedCount) {
+        Box<TestEntity> box = getTestEntityBox();
         box.put(new TestEntity());
-        assertEquals(1, box.count());
+        assertEquals(expectedCount, box.count());
     }
 
     @Test