Migrate from deprecated finalization API to Java 9+ API (#8486)

* Migrate from deprecated finalization API to Java 9+ API

* Use non-deprecated JTH method

core/src/main/java/hudson/util/AtomicFileWriter.java

@@ -30,6 +30,7 @@
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.Writer;
+import java.lang.ref.Cleaner;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.AtomicMoveNotSupportedException;
@@ -55,6 +56,9 @@ public class AtomicFileWriter extends Writer {
 
     private static final Logger LOGGER = Logger.getLogger(AtomicFileWriter.class.getName());
 
+    private static final Cleaner CLEANER = Cleaner.create(
+            new NamingThreadFactory(new DaemonThreadFactory(), AtomicFileWriter.class.getName() + ".cleaner"));
+
     private static /* final */ boolean DISABLE_FORCED_FLUSH = SystemProperties.getBoolean(
             AtomicFileWriter.class.getName() + ".DISABLE_FORCED_FLUSH");
 
@@ -64,7 +68,7 @@ public class AtomicFileWriter extends Writer {
         }
     }
 
-    private final Writer core;
+    private final FileChannelWriter core;
     private final Path tmpPath;
     private final Path destPath;
 
@@ -151,6 +155,8 @@ public AtomicFileWriter(@NonNull Path destinationPath, @NonNull Charset charset,
         }
 
         core = new FileChannelWriter(tmpPath, charset, integrityOnFlush, integrityOnClose, StandardOpenOption.WRITE, StandardOpenOption.CREATE);
+
+        CLEANER.register(this, new CleanupChecker(core, tmpPath, destPath));
     }
 
     @Override
@@ -185,7 +191,12 @@ public void close() throws IOException {
      * the {@link #commit()} is called, to simplify coding.
      */
     public void abort() throws IOException {
-        closeAndDeleteTempFile();
+        // One way or another, the temporary file should be deleted.
+        try {
+            close();
+        } finally {
+            Files.deleteIfExists(tmpPath);
+        }
     }
 
     public void commit() throws IOException {
@@ -225,21 +236,32 @@ public void commit() throws IOException {
         }
     }
 
-    @Override
-    protected void finalize() throws Throwable {
-        try {
-            closeAndDeleteTempFile();
-        } finally {
-            super.finalize();
+    private static final class CleanupChecker implements Runnable {
+        private final FileChannelWriter core;
+        private final Path tmpPath;
+        private final Path destPath;
+
+        CleanupChecker(final FileChannelWriter core, final Path tmpPath, final Path destPath) {
+            this.core = core;
+            this.tmpPath = tmpPath;
+            this.destPath = destPath;
         }
-    }
 
-    private void closeAndDeleteTempFile() throws IOException {
-        // one way or the other, temporary file should be deleted.
-        try {
-            close();
-        } finally {
-            Files.deleteIfExists(tmpPath);
+        @Override
+        public void run() {
+            if (core.isOpen()) {
+                LOGGER.log(Level.WARNING, "AtomicFileWriter for " + destPath + " was not closed before being released");
+                try {
+                    core.close();
+                } catch (IOException e) {
+                    LOGGER.log(Level.WARNING, "Failed to close " + tmpPath + " for destination file " + destPath, e);
+                }
+            }
+            try {
+                Files.deleteIfExists(tmpPath);
+            } catch (IOException e) {
+                LOGGER.log(Level.WARNING, "Failed to delete temporary file " + tmpPath + " for destination file " + destPath, e);
+            }
         }
     }
 

core/src/main/java/hudson/util/FileChannelWriter.java

@@ -5,6 +5,7 @@
 import java.io.Writer;
 import java.nio.ByteBuffer;
 import java.nio.CharBuffer;
+import java.nio.channels.Channel;
 import java.nio.channels.FileChannel;
 import java.nio.charset.Charset;
 import java.nio.file.OpenOption;
@@ -26,7 +27,7 @@
  * @see <a href="https://github.com/jenkinsci/jenkins/pull/2548">PR-2548</a>
  */
 @Restricted(NoExternalUse.class)
-public class FileChannelWriter extends Writer {
+public class FileChannelWriter extends Writer implements Channel {
 
     private static final Logger LOGGER = Logger.getLogger(FileChannelWriter.class.getName());
 
@@ -82,6 +83,11 @@ public void flush() throws IOException {
         }
     }
 
+    @Override
+    public boolean isOpen() {
+        return channel.isOpen();
+    }
+
     @Override
     public void close() throws IOException {
         if (channel.isOpen()) {

core/src/test/java/hudson/util/AtomicFileWriterTest.java

@@ -22,6 +22,7 @@
 import java.nio.file.Paths;
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.Set;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
@@ -72,6 +73,11 @@ public void setUp() throws IOException {
         afw = new AtomicFileWriter(af.toPath(), Charset.defaultCharset());
     }
 
+    @After
+    public void tearDown() throws IOException {
+        afw.abort();
+    }
+
     @Test
     public void symlinkToDirectory() throws Exception {
         assumeFalse(Functions.isWindows());
@@ -83,7 +89,7 @@ public void symlinkToDirectory() throws Exception {
 
         final Path childFileInSymlinkToDir = Paths.get(zeSymlink.toString(), "childFileInSymlinkToDir");
 
-        new AtomicFileWriter(childFileInSymlinkToDir, StandardCharsets.UTF_8);
+        new AtomicFileWriter(childFileInSymlinkToDir, StandardCharsets.UTF_8).abort();
     }
 
     @Test

test/src/test/java/hudson/util/AtomicFileWriterTest.java

@@ -0,0 +1,63 @@
+package hudson.util;
+
+import static org.awaitility.Awaitility.await;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.lang.ref.WeakReference;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.LoggerRule;
+import org.jvnet.hudson.test.MemoryAssert;
+
+public class AtomicFileWriterTest {
+    @Rule
+    public TemporaryFolder tmp = new TemporaryFolder();
+
+    @Rule
+    public LoggerRule logging =
+            new LoggerRule().record(AtomicFileWriter.class, Level.WARNING).capture(100);
+
+    @Test
+    public void noResourceLeak() throws IOException {
+        Path destPath = tmp.newFolder().toPath().resolve("file");
+        AtomicFileWriter writer = new AtomicFileWriter(destPath, StandardCharsets.UTF_8);
+        Path tmpPath = writer.getTemporaryPath();
+        assertTrue(Files.exists(tmpPath));
+        assertFalse(Files.exists(destPath));
+        try {
+            writer.commit();
+        } finally {
+            writer.abort();
+        }
+        assertFalse(Files.exists(tmpPath));
+        assertTrue(Files.exists(destPath));
+        assertThat(logging.getMessages(), empty());
+    }
+
+    @Test
+    public void resourceLeak() throws IOException {
+        Path destPath = tmp.newFolder().toPath().resolve("file");
+        WeakReference<AtomicFileWriter> ref =
+                new WeakReference<>(new AtomicFileWriter(destPath, StandardCharsets.UTF_8));
+        Path tmpPath = ref.get().getTemporaryPath();
+        assertTrue(Files.exists(tmpPath));
+        assertFalse(Files.exists(destPath));
+        MemoryAssert.assertGC(ref, false);
+        await().atMost(30, TimeUnit.SECONDS).until(() -> !Files.exists(tmpPath));
+        assertFalse(Files.exists(destPath));
+        assertThat(
+                logging.getMessages(),
+                contains("AtomicFileWriter for " + destPath + " was not closed before being released"));
+    }
+}