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
index 353a25156cc4940613a8a2a1a55b7ef8fb6c41fc..029464a82b0c83b2a89d93e4bf252a6fd5d3adb0 100644 (file)
@@ -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,11 +15,11 @@ 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 javax.annotation.concurrent.GuardedBy;
-import javax.annotation.concurrent.ThreadSafe;
+import org.checkerframework.checker.lock.qual.GuardedBy;
+import org.checkerframework.checker.lock.qual.Holding;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.slf4j.Logger;
@@ -35,22 +31,14 @@ import org.slf4j.LoggerFactory;
  *
  * @author Thomas Pantelis
  */
-@ThreadSafe
 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;
@@ -64,6 +52,9 @@ public class FileBackedOutputStream extends OutputStream {
     @GuardedBy("this")
     private File file;
 
+    @GuardedBy("this")
+    private Cleanable fileCleanup;
+
     @GuardedBy("this")
     private ByteSource source;
 
@@ -77,7 +68,7 @@ public class FileBackedOutputStream extends OutputStream {
      * @param fileDirectory the directory in which to create the file if needed. If null, the default temp file
      *                      location is used.
      */
-    public FileBackedOutputStream(int fileThreshold, @Nullable String fileDirectory) {
+    public FileBackedOutputStream(final int fileThreshold, @Nullable final String fileDirectory) {
         this.fileThreshold = fileThreshold;
         this.fileDirectory = fileDirectory;
     }
@@ -118,19 +109,19 @@ public class FileBackedOutputStream extends OutputStream {
     @Override
     @SuppressFBWarnings(value = "VO_VOLATILE_INCREMENT", justification = "Findbugs erroneously complains that the "
         + "increment of count needs to be atomic even though it is inside a synchronized block.")
-    public synchronized void write(int value) throws IOException {
+    public synchronized void write(final int value) throws IOException {
         possiblySwitchToFile(1);
         out.write(value);
         count++;
     }
 
     @Override
-    public synchronized void write(byte[] bytes) throws IOException {
+    public synchronized void write(final byte[] bytes) throws IOException {
         write(bytes, 0, bytes.length);
     }
 
     @Override
-    public synchronized void write(byte[] bytes, int off, int len) throws IOException {
+    public synchronized void write(final byte[] bytes, final int off, final int len) throws IOException {
         possiblySwitchToFile(len);
         out.write(bytes, off, len);
         count += len;
@@ -161,26 +152,15 @@ 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;
     }
 
-    @GuardedBy("this")
+    @Holding("this")
     private void closeQuietly() {
         try {
             close();
@@ -192,48 +172,50 @@ public class FileBackedOutputStream extends OutputStream {
     /**
      * Checks if writing {@code len} bytes would go over threshold, and switches to file buffering if so.
      */
-    @GuardedBy("this")
-    private void possiblySwitchToFile(int len) throws IOException {
+    @Holding("this")
+    private void possiblySwitchToFile(final int len) throws IOException {
         if (out == null) {
             throw new IOException("Stream already closed");
         }
 
         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(File file) {
+    private static void deleteFile(final File file) {
+        LOG.debug("Deleting temp file {}", file);
         if (!file.delete()) {
             LOG.warn("Could not delete temp file {}", file);
         }
@@ -251,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(FileBackedOutputStream referent, 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);
-            }
-        }
-    }
 }