Use java.lang.ref.Cleaner in controller.cluster.io

[controller.git] / opendaylight / md-sal / sal-clustering-commons / src / main / java / org / opendaylight / controller / cluster / io / FileBackedOutputStream.java

diff --git a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/io/FileBackedOutputStream.java b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/io/FileBackedOutputStream.java
index 970b06f5c2fc3d4995100920efc0227d7ce8cd1c..029464a82b0c83b2a89d93e4bf252a6fd5d3adb0 100644 (file)
--- a/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/io/FileBackedOutputStream.java
+++ b/opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/io/FileBackedOutputStream.java
@@ -7,10 +7,6 @@
  */
 package org.opendaylight.controller.cluster.io;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.FinalizablePhantomReference;
-import com.google.common.base.FinalizableReferenceQueue;
-import com.google.common.collect.Sets;
 import com.google.common.io.ByteSource;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.ByteArrayInputStream;
@@ -19,9 +15,9 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.lang.ref.Cleaner;
+import java.lang.ref.Cleaner.Cleanable;
 import java.nio.file.Files;
-import java.util.Iterator;
-import java.util.Set;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.checkerframework.checker.lock.qual.Holding;
 import org.eclipse.jdt.annotation.NonNull;
@@ -39,17 +35,10 @@ public class FileBackedOutputStream extends OutputStream {
     private static final Logger LOG = LoggerFactory.getLogger(FileBackedOutputStream.class);
 
     /**
-     * This stores the Cleanup PhantomReference instances statically. This is necessary because PhantomReferences
-     * need a hard reference so they're not garbage collected. Once finalized, the Cleanup PhantomReference removes
-     * itself from this map and thus becomes eligible for garbage collection.
+     * A Cleaner instance responsible for deleting any files which may be lost due to us not being cleaning up
+     * temporary files.
      */
-    @VisibleForTesting
-    static final Set<Cleanup> REFERENCE_CACHE = Sets.newConcurrentHashSet();
-
-    /**
-     * Used as the ReferenceQueue for the Cleanup PhantomReferences.
-     */
-    private static final FinalizableReferenceQueue REFERENCE_QUEUE = new FinalizableReferenceQueue();
+    private static final Cleaner FILE_CLEANER = Cleaner.create();
 
     private final int fileThreshold;
     private final String fileDirectory;
@@ -63,6 +52,9 @@ public class FileBackedOutputStream extends OutputStream {
     @GuardedBy("this")
     private File file;
 
+    @GuardedBy("this")
+    private Cleanable fileCleanup;
+
     @GuardedBy("this")
     private ByteSource source;
 
@@ -160,23 +152,12 @@ public class FileBackedOutputStream extends OutputStream {
      */
     public synchronized void cleanup() {
         LOG.debug("In cleanup");
-
         closeQuietly();
-
-        if (file != null) {
-            Iterator<Cleanup> iter = REFERENCE_CACHE.iterator();
-            while (iter.hasNext()) {
-                if (file.equals(iter.next().file)) {
-                    iter.remove();
-                    break;
-                }
-            }
-
-            LOG.debug("cleanup - deleting temp file {}", file);
-
-            deleteFile(file);
-            file = null;
+        if (fileCleanup != null) {
+            fileCleanup.clean();
         }
+        // Already deleted above
+        file = null;
     }
 
     @Holding("this")
@@ -198,41 +179,43 @@ public class FileBackedOutputStream extends OutputStream {
         }
 
         if (file == null && memory.getCount() + len > fileThreshold) {
-            File temp = File.createTempFile("FileBackedOutputStream", null,
+            final File temp = File.createTempFile("FileBackedOutputStream", null,
                     fileDirectory == null ? null : new File(fileDirectory));
             temp.deleteOnExit();
+            final Cleaner.Cleanable cleanup = FILE_CLEANER.register(this, () -> deleteFile(temp));
 
             LOG.debug("Byte count {} has exceeded threshold {} - switching to file: {}", memory.getCount() + len,
                     fileThreshold, temp);
 
-            OutputStream transfer = null;
+            final OutputStream transfer;
             try {
                 transfer = Files.newOutputStream(temp.toPath());
-                transfer.write(memory.getBuffer(), 0, memory.getCount());
-                transfer.flush();
-
-                // We've successfully transferred the data; switch to writing to file
-                out = transfer;
-                file = temp;
-                memory = null;
-
-                new Cleanup(this, file);
-            } catch (IOException e) {
-                if (transfer != null) {
+                try {
+                    transfer.write(memory.getBuffer(), 0, memory.getCount());
+                    transfer.flush();
+                } catch (IOException e) {
                     try {
                         transfer.close();
                     } catch (IOException ex) {
                         LOG.debug("Error closing temp file {}", temp, ex);
                     }
+                    throw e;
                 }
-
-                deleteFile(temp);
+            } catch (IOException e) {
+                cleanup.clean();
                 throw e;
             }
+
+            // We've successfully transferred the data; switch to writing to file
+            out = transfer;
+            file = temp;
+            fileCleanup = cleanup;
+            memory = null;
         }
     }
 
     private static void deleteFile(final File file) {
+        LOG.debug("Deleting temp file {}", file);
         if (!file.delete()) {
             LOG.warn("Could not delete temp file {}", file);
         }
@@ -250,30 +233,4 @@ public class FileBackedOutputStream extends OutputStream {
             return count;
         }
     }
-
-    /**
-     * PhantomReference that deletes the temp file when the FileBackedOutputStream is garbage collected.
-     */
-    private static class Cleanup extends FinalizablePhantomReference<FileBackedOutputStream> {
-        private final File file;
-
-        Cleanup(final FileBackedOutputStream referent, final File file) {
-            super(referent, REFERENCE_QUEUE);
-            this.file = file;
-
-            REFERENCE_CACHE.add(this);
-
-            LOG.debug("Added Cleanup for temp file {}", file);
-        }
-
-        @Override
-        public void finalizeReferent() {
-            LOG.debug("In finalizeReferent");
-
-            if (REFERENCE_CACHE.remove(this)) {
-                LOG.debug("finalizeReferent - deleting temp file {}", file);
-                deleteFile(file);
-            }
-        }
-    }
 }