OAK-5438 - Watch out for exception midway through the construction an…

…d destruction of the FileStore

git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/oak/trunk@1778158 13f79535-47bb-0310-9956-ffa450edef68

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java

@@ -74,6 +74,7 @@
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
+import com.google.common.io.Closer;
 import org.apache.jackrabbit.oak.api.jmx.CacheStatsMBean;
 import org.apache.jackrabbit.oak.plugins.blob.ReferenceCollector;
 import org.apache.jackrabbit.oak.segment.Compactor;
@@ -462,28 +463,34 @@ public void close() {
 
         try {
             flush();
-            revisions.close();
-            fileStoreLock.writeLock().lock();
-            try {
-                closeAndLogOnFail(tarWriter);
-
-                List<TarReader> list = readers;
-                readers = newArrayList();
-                for (TarReader reader : list) {
-                    closeAndLogOnFail(reader);
-                }
+        } catch (IOException e) {
+            log.warn("Unable to flush the store", e);
+        }
 
-                if (lock != null) {
+        Closer closer = Closer.create();
+        closer.register(revisions);
+        fileStoreLock.writeLock().lock();
+        try {
+            if (lock != null) {
+                try {
                     lock.release();
+                } catch (IOException e) {
+                    log.warn("Unable to release the file lock", e);
                 }
-                closeAndLogOnFail(lockFile);
-            } finally {
-                fileStoreLock.writeLock().unlock();
             }
-        } catch (IOException e) {
-            throw new RuntimeException(
-                    "Failed to close the TarMK at " + directory, e);
+            closer.register(lockFile);
+
+            List<TarReader> list = readers;
+            readers = newArrayList();
+            for (TarReader reader : list) {
+                closer.register(reader);
+            }
+
+            closer.register(tarWriter);
+        } finally {
+            fileStoreLock.writeLock().unlock();
         }
+        closeAndLogOnFail(closer);
 
         // Try removing pending files in case the scheduler didn't have a chance to run yet
         fileReaper.reap();

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStoreBuilder.java

@@ -308,7 +308,19 @@ public FileStore build() throws InvalidFileStoreVersionException, IOException {
         directory.mkdirs();
         TarRevisions revisions = new TarRevisions(directory);
         LOG.info("Creating file store {}", this);
-        return new FileStore(this).bind(revisions);
+        FileStore store;
+        try {
+            store = new FileStore(this);
+        } catch (InvalidFileStoreVersionException | IOException e) {
+            try {
+                revisions.close();
+            } catch (IOException re) {
+                LOG.warn("Unable to close TarRevisions", re);
+            }
+            throw e;
+        }
+        store.bind(revisions);
+        return store;
     }
 
     /**
@@ -335,7 +347,19 @@ public ReadOnlyFileStore buildReadOnly() throws InvalidFileStoreVersionException
         built = true;
         ReadOnlyRevisions revisions = new ReadOnlyRevisions(directory);
         LOG.info("Creating file store {}", this);
-        return new ReadOnlyFileStore(this).bind(revisions);
+        ReadOnlyFileStore store;
+        try {
+            store = new ReadOnlyFileStore(this);
+        } catch (InvalidFileStoreVersionException | IOException e) {
+            try {
+                revisions.close();
+            } catch (IOException re) {
+                LOG.warn("Unable to close ReadOnlyRevisions", re);
+            }
+            throw e;
+        }
+        store.bind(revisions);
+        return store;
     }
 
     @Nonnull

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/ReadOnlyFileStore.java

@@ -40,6 +40,7 @@
 
 import javax.annotation.Nonnull;
 
+import com.google.common.io.Closer;
 import org.apache.jackrabbit.oak.segment.RecordId;
 import org.apache.jackrabbit.oak.segment.Segment;
 import org.apache.jackrabbit.oak.segment.SegmentGraph.SegmentGraphVisitor;
@@ -69,8 +70,7 @@ public class ReadOnlyFileStore extends AbstractFileStore {
 
     private RecordId currentHead;
 
-    ReadOnlyFileStore(FileStoreBuilder builder)
-            throws InvalidFileStoreVersionException, IOException {
+    ReadOnlyFileStore(FileStoreBuilder builder) throws InvalidFileStoreVersionException, IOException {
         super(builder);
 
         Map<Integer, Map<Character, File>> map = collectFiles(directory);
@@ -94,8 +94,7 @@ public class ReadOnlyFileStore extends AbstractFileStore {
                 memoryMapping);
     }
 
-    ReadOnlyFileStore bind(@Nonnull ReadOnlyRevisions revisions)
-            throws IOException {
+    ReadOnlyFileStore bind(@Nonnull ReadOnlyRevisions revisions) throws IOException {
         this.revisions = revisions;
         this.revisions.bind(this);
         currentHead = revisions.getHead();
@@ -202,15 +201,12 @@ public Segment call() throws Exception {
 
     @Override
     public void close() {
-        try {
-            revisions.close();
-            for (TarReader reader : readers) {
-                closeAndLogOnFail(reader);
-            }
-        } catch (IOException e) {
-            throw new RuntimeException("Failed to close the TarMK at "
-                    + directory, e);
+        Closer closer = Closer.create();
+        for (TarReader r : readers) {
+            closer.register(r);
         }
+        closer.register(revisions);
+        closeAndLogOnFail(closer);
         System.gc(); // for any memory-mappings that are no longer used
         log.info("TarMK closed: {}", directory);
     }