BoxStore fix: close resources and remove path from openFiles if an exception is thrown in constructor, add test usePreviousCommitAfterFileCorruptException()

Author: greenrobot

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

@@ -225,47 +225,52 @@ public static boolean isObjectBrowserAvailable() {
         canonicalPath = getCanonicalPath(directory);
         verifyNotAlreadyOpen(canonicalPath);
 
-        handle = nativeCreateWithFlatOptions(builder.buildFlatStoreOptions(canonicalPath), builder.model);
-        int debugFlags = builder.debugFlags;
-        if (debugFlags != 0) {
-            debugTxRead = (debugFlags & DebugFlags.LOG_TRANSACTIONS_READ) != 0;
-            debugTxWrite = (debugFlags & DebugFlags.LOG_TRANSACTIONS_WRITE) != 0;
-        } else {
-            debugTxRead = debugTxWrite = false;
-        }
-        debugRelations = builder.debugRelations;
+        try {
+            handle = nativeCreateWithFlatOptions(builder.buildFlatStoreOptions(canonicalPath), builder.model);
+            int debugFlags = builder.debugFlags;
+            if (debugFlags != 0) {
+                debugTxRead = (debugFlags & DebugFlags.LOG_TRANSACTIONS_READ) != 0;
+                debugTxWrite = (debugFlags & DebugFlags.LOG_TRANSACTIONS_WRITE) != 0;
+            } else {
+                debugTxRead = debugTxWrite = false;
+            }
+            debugRelations = builder.debugRelations;
 
-        for (EntityInfo<?> entityInfo : builder.entityInfoList) {
-            try {
-                dbNameByClass.put(entityInfo.getEntityClass(), entityInfo.getDbName());
-                int entityId = nativeRegisterEntityClass(handle, entityInfo.getDbName(), entityInfo.getEntityClass());
-                entityTypeIdByClass.put(entityInfo.getEntityClass(), entityId);
-                classByEntityTypeId.put(entityId, entityInfo.getEntityClass());
-                propertiesByClass.put(entityInfo.getEntityClass(), entityInfo);
-                for (Property<?> property : entityInfo.getAllProperties()) {
-                    if (property.customType != null) {
-                        if (property.converterClass == null) {
-                            throw new RuntimeException("No converter class for custom type of " + property);
+            for (EntityInfo<?> entityInfo : builder.entityInfoList) {
+                try {
+                    dbNameByClass.put(entityInfo.getEntityClass(), entityInfo.getDbName());
+                    int entityId = nativeRegisterEntityClass(handle, entityInfo.getDbName(), entityInfo.getEntityClass());
+                    entityTypeIdByClass.put(entityInfo.getEntityClass(), entityId);
+                    classByEntityTypeId.put(entityId, entityInfo.getEntityClass());
+                    propertiesByClass.put(entityInfo.getEntityClass(), entityInfo);
+                    for (Property<?> property : entityInfo.getAllProperties()) {
+                        if (property.customType != null) {
+                            if (property.converterClass == null) {
+                                throw new RuntimeException("No converter class for custom type of " + property);
+                            }
+                            nativeRegisterCustomType(handle, entityId, 0, property.dbName, property.converterClass,
+                                    property.customType);
                         }
-                        nativeRegisterCustomType(handle, entityId, 0, property.dbName, property.converterClass,
-                                property.customType);
                     }
+                } catch (RuntimeException e) {
+                    throw new RuntimeException("Could not setup up entity " + entityInfo.getEntityClass(), e);
                 }
-            } catch (RuntimeException e) {
-                throw new RuntimeException("Could not setup up entity " + entityInfo.getEntityClass(), e);
             }
-        }
-        int size = classByEntityTypeId.size();
-        allEntityTypeIds = new int[size];
-        long[] entityIdsLong = classByEntityTypeId.keys();
-        for (int i = 0; i < size; i++) {
-            allEntityTypeIds[i] = (int) entityIdsLong[i];
-        }
+            int size = classByEntityTypeId.size();
+            allEntityTypeIds = new int[size];
+            long[] entityIdsLong = classByEntityTypeId.keys();
+            for (int i = 0; i < size; i++) {
+                allEntityTypeIds[i] = (int) entityIdsLong[i];
+            }
 
-        objectClassPublisher = new ObjectClassPublisher(this);
+            objectClassPublisher = new ObjectClassPublisher(this);
 
-        failedReadTxAttemptCallback = builder.failedReadTxAttemptCallback;
-        queryAttempts = Math.max(builder.queryAttempts, 1);
+            failedReadTxAttemptCallback = builder.failedReadTxAttemptCallback;
+            queryAttempts = Math.max(builder.queryAttempts, 1);
+        } catch (RuntimeException runtimeException) {
+            close();  // Proper clean up, e.g. delete native handle, remove this path from openFiles
+            throw runtimeException;
+        }
     }
 
     static String getCanonicalPath(File directory) {

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

@@ -183,6 +183,27 @@ public void usePreviousCommitWithCorruptFile() throws IOException {
         assertTrue(store.deleteAllFiles());
     }
 
+    @Test
+    public void usePreviousCommitAfterFileCorruptException() throws IOException {
+        File dir = prepareTempDir("object-store-test-corrupted");
+        prepareBadDataFile(dir);
+        builder = BoxStoreBuilder.createDebugWithoutModel().directory(dir);
+        builder.validateOnOpen(ValidateOnOpenMode.Full);
+        try {
+            store = builder.build();
+            fail("Should have thrown");
+        } catch (FileCorruptException e) {
+            builder.usePreviousCommit();
+            store = builder.build();
+        }
+
+        String diagnoseString = store.diagnose();
+        assertTrue(diagnoseString.contains("entries=2"));
+        store.validate(0, true);
+        store.close();
+        assertTrue(store.deleteAllFiles());
+    }
+
     private File prepareBadDataFile(File dir) throws IOException {
         assertTrue(dir.mkdir());
         File badDataFile = new File(dir, "data.mdb");