Merge branch '240-boxstore-openfiles-deadlock' into 'dev'

BoxStore: when constructing, actually wait on finalization of other instances #240

See merge request objectbox/objectbox-java!161

Author: greenrobot-team

CHANGELOG.md

@@ -6,6 +6,11 @@ For more insights into what changed in the ObjectBox C++ core, [check the Object
 
 ## 4.3.2 - in development
 
+- When re-creating a `BoxStore` for the same directory and `close()` wasn't called on the previous instance, don't throw
+  an "Another BoxStore is still open for this directory" exception. Note that calling `close()` *is recommended* before 
+  creating a new instance. [#1201](https://github.com/objectbox/objectbox-java/issues/1201)
+- When using `BoxStoreBuilder.buildDefault()`, don't leak Store when setting as default fails.
+
 ## 4.3.1 - 2025-08-12
 
 - Requires at least Kotlin compiler and standard library 1.7.

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

@@ -359,16 +359,34 @@ static String getCanonicalPath(File directory) {
     }
 
     static void verifyNotAlreadyOpen(String canonicalPath) {
+        // Call isFileOpen before, but without checking the result, to try to close any unreferenced instances where
+        // it was forgotten to close them.
+        // Only obtain the lock on openFiles afterward as the checker thread created by isFileOpen needs to obtain it to
+        // do anything.
+        isFileOpen(canonicalPath);
         synchronized (openFiles) {
-            isFileOpen(canonicalPath); // for retries
             if (!openFiles.add(canonicalPath)) {
-                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.");
+                throw new DbException("Another BoxStore is still open for this directory (" + canonicalPath +
+                        "). Make sure the existing instance is explicitly closed before creating a new one.");
             }
         }
     }
 
-    /** Also retries up to 500ms to improve GC race condition situation. */
+    /**
+     * Returns if the given path is in {@link #openFiles}.
+     * <p>
+     * If it is, (creates and) briefly waits on an existing "checker" thread before checking again and returning the
+     * result.
+     * <p>
+     * The "checker" thread locks {@link #openFiles} while it triggers garbage collection and finalization in this Java
+     * Virtual Machine to try to close any unreferenced BoxStore instances. These might exist if it was forgotten to
+     * close them explicitly.
+     * <p>
+     * Note that the finalization mechanism relied upon here is scheduled for removal in future versions of Java and may
+     * already be disabled depending on JVM configuration.
+     *
+     * @see #finalize()
+     */
     static boolean isFileOpen(final String canonicalPath) {
         synchronized (openFiles) {
             if (!openFiles.contains(canonicalPath)) return false;
@@ -522,9 +540,13 @@ public long getDbSizeOnDisk() {
     }
 
     /**
-     * Closes this if this is finalized.
+     * Calls {@link #close()}.
+     * <p>
+     * It is strongly recommended to instead explicitly close a Store and not rely on finalization. For example, on
+     * Android finalization has a timeout that might be exceeded if closing needs to wait on transactions to finish.
      * <p>
-     * Explicitly call {@link #close()} instead to avoid expensive finalization.
+     * Also finalization is scheduled for removal in future versions of Java and may already be disabled depending on
+     * JVM configuration (see documentation on super method).
      */
     @SuppressWarnings("deprecation") // finalize()
     @Override

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

@@ -679,7 +679,13 @@ static File getDbDir(@Nullable File baseDirectoryOrNull, @Nullable String nameOr
      */
     public BoxStore buildDefault() {
         BoxStore store = build();
-        BoxStore.setDefault(store);
+        try {
+            BoxStore.setDefault(store);
+        } catch (IllegalStateException e) {
+            // Clean up the new store if it can't be set as default
+            store.close();
+            throw e;
+        }
         return store;
     }
 

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

@@ -162,10 +162,6 @@ protected Box<TestEntity> getTestEntityBox() {
 
     @After
     public void tearDown() {
-        // Collect dangling Cursors and TXs before store closes
-        System.gc();
-        System.runFinalization();
-
         if (store != null) {
             try {
                 store.close();

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

@@ -174,6 +174,19 @@ public void openSamePath_afterClose_works() {
         assertEquals(0, BoxStore.nativeGloballyActiveEntityTypes());
     }
 
+    @Test
+    public void openSamePath_closedByFinalizer_works() {
+        System.out.println("Removing reference to " + store);
+        store = null;
+
+        // 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, see notes on BoxStore.finalize().
+        BoxStore store2 = createBoxStore();
+        store2.close();
+        System.out.println("Closed " + store2);
+    }
+
     @Test
     public void testOpenTwoBoxStoreTwoFiles() {
         File boxStoreDir2 = new File(boxStoreDir.getAbsolutePath() + "-2");