Transaction: close write transactions even if store is closing #282

Otherwise, the native store would wait forever for the write transaction

Author: greenrobot

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

@@ -244,8 +244,11 @@ public static boolean isSyncServerAvailable() {
     private final PrintStream errorOutputStream;
     private final File directory;
     private final String canonicalPath;
+
     /** Reference to the native store. Should probably get through {@link #getNativeStore()} instead. */
     volatile private long handle;
+    volatile private boolean nativeStoreDestroyed = false;
+
     private final Map<Class<?>, String> dbNameByClass = new HashMap<>();
     private final Map<Class<?>, Integer> entityTypeIdByClass = new HashMap<>();
     private final Map<Class<?>, EntityInfo<?>> propertiesByClass = new HashMap<>();
@@ -724,6 +727,7 @@ public void close() {
                 handle = 0;
                 if (handleToDelete != 0) { // failed before native handle was created?
                     nativeDelete(handleToDelete);
+                    nativeStoreDestroyed = true;
                 }
             }
         }
@@ -1414,6 +1418,18 @@ public boolean isNativeStoreClosed() {
         return handle == 0;
     }
 
+    /**
+     * For internal use only. This API might change or be removed with a future release.
+     * <p>
+     * Returns if the native Store was completely destroyed.
+     * <p>
+     * This is {@code true} shortly after {@link #close()} was called and {@link #isClosed()} returns {@code true}.
+     */
+    @Internal
+    public boolean isNativeStoreDestroyed() {
+        return nativeStoreDestroyed;
+    }
+
     /**
      * Returns the {@link SyncClient} associated with this store. To create one see {@link io.objectbox.sync.Sync Sync}.
      */

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

@@ -101,7 +101,8 @@ public synchronized void close() {
             closed = true;
             store.unregisterTransaction(this);
 
-            if (!nativeIsOwnerThread(transaction)) {
+            boolean isOwnerThread = nativeIsOwnerThread(transaction);
+            if (!isOwnerThread) {
                 boolean isActive = nativeIsActive(transaction);
                 boolean isRecycled = nativeIsRecycled(transaction);
                 if (isActive || isRecycled) {
@@ -126,6 +127,27 @@ public synchronized void close() {
             // TODO not destroying is probably only a small leak on rare occasions, but still could be fixed
             if (!store.isNativeStoreClosed()) {
                 nativeDestroy(transaction);
+            } else {
+                String threadType = isOwnerThread ? "owner thread" : "non-owner thread";
+                if (readOnly) {
+                    // Minor leak, but still print it so we can check logs: it should only happen occasionally.
+                    // We cannot rely on the store waiting for read transactions; it only waits for a limited time.
+                    System.out.println("Info: closing read transaction after store was closed (should be avoided) in " +
+                            threadType);
+                    System.out.flush();
+                } else {  // write transaction
+                    System.out.println("WARN: closing write transaction after store was closed (must be avoided) in " +
+                            threadType);
+                    System.out.flush();
+                    if (store.isNativeStoreDestroyed()) {
+                        // This is an internal validation: if this is a write-TX,
+                        // the (native) store will always wait for it, so it must not be destroyed yet.
+                        // If this ever happens, the above assumption is wrong, and it probably prevents a SIGSEGV.
+                        throw new IllegalStateException(
+                                "Cannot destroy write transaction for an already destroyed store");
+                    }
+                    nativeDestroy(transaction);
+                }
             }
         }
     }