Tests: try to confirm close() waiting forever on openFiles locked by checker thread #240

Author: greenrobot-team

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

@@ -358,10 +358,11 @@ static String getCanonicalPath(File directory) {
         }
     }
 
-    static void verifyNotAlreadyOpen(String canonicalPath) {
+    void verifyNotAlreadyOpen(String canonicalPath) {
         synchronized (openFiles) {
-            isFileOpen(canonicalPath); // for retries
+            boolean fileOpen = isFileOpen(canonicalPath); // for retries
             if (!openFiles.add(canonicalPath)) {
+                System.out.println("verifyNotAlreadyOpen failed for " + this + " (fileOpen=" + fileOpen + ")");
                 throw new DbException("Another BoxStore is still open for this directory: " + canonicalPath +
                         ". Hint: for most apps it's recommended to keep a BoxStore for the app's life time.");
             }
@@ -377,7 +378,8 @@ static boolean isFileOpen(final String canonicalPath) {
         if (checkerThread == null || !checkerThread.isAlive()) {
             // Use a thread to avoid finalizers that block us
             checkerThread = new Thread(() -> {
-                isFileOpenSync(canonicalPath, true);
+                boolean fileOpen = isFileOpenSync(canonicalPath, true);
+                System.out.println("checkerThread retries completed (fileOpen=" + fileOpen + ")");
                 BoxStore.openFilesCheckerThread = null; // Clean ref to itself
             });
             checkerThread.setDaemon(true);
@@ -403,6 +405,7 @@ static boolean isFileOpenSync(String canonicalPath, boolean runFinalization) {
             int tries = 0;
             while (tries < 5 && openFiles.contains(canonicalPath)) {
                 tries++;
+                System.out.println("isFileOpenSync calling System.gc() and System.runFinalization()");
                 System.gc();
                 if (runFinalization && tries > 1) System.runFinalization();
                 System.gc();
@@ -529,6 +532,7 @@ public long getDbSizeOnDisk() {
     @SuppressWarnings("deprecation") // finalize()
     @Override
     protected void finalize() throws Throwable {
+        System.out.println("finalize() called for " + this + " by " + Thread.currentThread());
         close();
         super.finalize();
     }
@@ -654,6 +658,7 @@ public boolean isReadOnly() {
      * are properly finished.
      */
     public void close() {
+        System.out.println("close() called for " + this + " (handle=" + handle + ")");
         boolean oldClosedState;
         synchronized (this) {
             oldClosedState = closed;
@@ -716,6 +721,7 @@ public void close() {
                 openFiles.notifyAll();
             }
         }
+        System.out.println("close() finished for " + this);
     }
 
     /** dump thread stacks if pool does not terminate promptly. */

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

@@ -16,6 +16,7 @@
 
 package io.objectbox;
 
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.function.ThrowingRunnable;
 
@@ -174,6 +175,26 @@ public void openSamePath_afterClose_works() {
         assertEquals(0, BoxStore.nativeGloballyActiveEntityTypes());
     }
 
+    @Ignore("Fails due to related issue")
+    @Test
+    public void openSamePath_closedByFinalizer_works() {
+        System.out.println("Removing reference to " + store);
+        store = null;
+
+        // TODO Can't, at least on a JVM 21, confirm that the gc and runFinalization calls are blocking close() from
+        //  accessing openFiles.
+        //  Instead, found another issue where openFiles appears to still contain the db directory on this thread but
+        //  not for the checker thread.
+
+        // When another Store is still open using the same path, a checker thread is started that periodically triggers
+        // garbage collection and finalization in the VM, which here should close store and allow store2 to be opened.
+        // Note that user code should not rely on this (for ex. finalizers on Android have a timeout, which might be
+        // exceeded if closing the store takes long, because it needs to wait on transactions to finish), the store
+        // should be explicitly closed instead.
+        BoxStore store2 = createBoxStore();
+        store2.close();
+    }
+
     @Test
     public void testOpenTwoBoxStoreTwoFiles() {
         File boxStoreDir2 = new File(boxStoreDir.getAbsolutePath() + "-2");