Ensure WatchService is cleaned speedily

Rather than registering a shutdown hook, we prefer to use a cleaner.
While this allocates a thread, it is not a privileged action and we
kill the service as soon as we are GC'd.

Change-Id: I04e5fa2cba57f98a20ee038eb1d0be6acc21b9fa
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/DefaultFileWatcher.java b/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/DefaultFileWatcher.java
index 4fda85d42a15605c620b443d8df9d64f771e3ecf..6ccdb62c45f0cfdf3910cd5236cd1447df840dfc 100644 (file)
--- a/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/DefaultFileWatcher.java
+++ b/config-loader/config-loader-impl/src/main/java/org/opendaylight/bgpcep/config/loader/impl/DefaultFileWatcher.java
@@ -9,9 +9,12 @@ package org.opendaylight.bgpcep.config.loader.impl;
 
 import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE;
 import static java.nio.file.StandardWatchEventKinds.OVERFLOW;
+import static java.util.Objects.requireNonNull;
 
 import java.io.File;
 import java.io.IOException;
+import java.lang.ref.Cleaner;
+import java.lang.ref.Cleaner.Cleanable;
 import java.nio.file.FileSystems;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -19,6 +22,7 @@ import java.nio.file.WatchService;
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import javax.inject.Singleton;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.concepts.AbstractRegistration;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
@@ -29,15 +33,37 @@ import org.slf4j.LoggerFactory;
 @Singleton
 @Component(immediate = true, service = FileWatcher.class)
 public final class DefaultFileWatcher extends AbstractRegistration implements FileWatcher {
+    // TODO: this allocates a thread to the cleaning action, which really is a safety net. if we get some centralized
+    //       place for low-usage ODL threads we want to use that.
+    private static final Cleaner CLEANER = Cleaner.create();
+
+    private static final class State implements Runnable {
+        final @NonNull WatchService watchService;
+
+        State(final WatchService watchService) {
+            this.watchService = requireNonNull(watchService);
+        }
+
+        @Override
+        public void run() {
+            try {
+                watchService.close();
+            } catch (IOException e) {
+                LOG.warn("Failed to close watch service", e);
+            }
+        }
+    }
+
     private static final Logger LOG = LoggerFactory.getLogger(DefaultFileWatcher.class);
     //BGPCEP config folder OS agnostic path
     private static final Path PATH = Paths.get("etc","opendaylight","bgpcep");
 
-    private final WatchService watchService;
+    private final State state;
+    private final Cleanable cleanable;
 
     public DefaultFileWatcher() throws IOException {
-        watchService = FileSystems.getDefault().newWatchService();
-        Runtime.getRuntime().addShutdownHook(new Thread(this::close));
+        state = new State(FileSystems.getDefault().newWatchService());
+        cleanable = CLEANER.register(this, state);
     }
 
     @Override
@@ -47,7 +73,7 @@ public final class DefaultFileWatcher extends AbstractRegistration implements Fi
 
     @Override
     public WatchService getWatchService() {
-        return watchService;
+        return state.watchService;
     }
 
     @Activate
@@ -61,7 +87,7 @@ public final class DefaultFileWatcher extends AbstractRegistration implements Fi
             }
         }
 
-        PATH.register(this.watchService, OVERFLOW, ENTRY_CREATE);
+        PATH.register(getWatchService(), OVERFLOW, ENTRY_CREATE);
         LOG.info("File Watcher service started");
     }
 
@@ -74,11 +100,7 @@ public final class DefaultFileWatcher extends AbstractRegistration implements Fi
 
     @Override
     protected void removeRegistration() {
-        try {
-            watchService.close();
-        } catch (IOException e) {
-            LOG.warn("Failed to close watch service", e);
-        }
+        cleanable.clean();
         LOG.info("File Watcher service stopped");
     }
 }