From 5c9edc9c8edfb2950bb7e163cb6fdc5064245f19 Mon Sep 17 00:00:00 2001 From: Christoph Date: Wed, 2 Sep 2020 12:28:05 +0200 Subject: [PATCH] Fix file update monitor shutdown (#6835) --- src/main/java/org/jabref/Globals.java | 1 + .../gui/collab/DatabaseChangeMonitor.java | 2 +- .../gui/util/DefaultFileUpdateMonitor.java | 21 ++++++++++++++++--- .../jabref/logic/util/DelayTaskThrottler.java | 21 ++++++++++++++++--- .../database/event/CoarseChangeFilter.java | 4 ++-- .../model/util/DummyFileUpdateMonitor.java | 10 +++++++-- .../jabref/model/util/FileUpdateMonitor.java | 5 +++++ 7 files changed, 53 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/jabref/Globals.java b/src/main/java/org/jabref/Globals.java index 8663a282b13..84c27f95a65 100644 --- a/src/main/java/org/jabref/Globals.java +++ b/src/main/java/org/jabref/Globals.java @@ -131,6 +131,7 @@ public static FileUpdateMonitor getFileUpdateMonitor() { public static void shutdownThreadPools() { TASK_EXECUTOR.shutdown(); + fileUpdateMonitor.shutdown(); JabRefExecutorService.INSTANCE.shutdownEverything(); } diff --git a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java b/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java index 6c1b8e2c55f..6181129601b 100644 --- a/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java +++ b/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java @@ -20,7 +20,7 @@ public class DatabaseChangeMonitor implements FileUpdateListener { private final BibDatabaseContext database; private final FileUpdateMonitor fileMonitor; private final List listeners; - private TaskExecutor taskExecutor; + private final TaskExecutor taskExecutor; public DatabaseChangeMonitor(BibDatabaseContext database, FileUpdateMonitor fileMonitor, TaskExecutor taskExecutor) { this.database = database; diff --git a/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java b/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java index 23dafbb137e..ed9d65633a6 100644 --- a/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java +++ b/src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java @@ -1,6 +1,7 @@ package org.jabref.gui.util; import java.io.IOException; +import java.nio.file.ClosedWatchServiceException; import java.nio.file.FileSystems; import java.nio.file.Path; import java.nio.file.StandardWatchEventKinds; @@ -8,6 +9,7 @@ import java.nio.file.WatchKey; import java.nio.file.WatchService; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import org.jabref.JabRefException; import org.jabref.model.util.FileUpdateListener; @@ -31,6 +33,7 @@ public class DefaultFileUpdateMonitor implements Runnable, FileUpdateMonitor { private final Multimap listeners = ArrayListMultimap.create(20, 4); private WatchService watcher; + private final AtomicBoolean notShutdown = new AtomicBoolean(true); private Optional filesystemMonitorFailure; @Override @@ -39,11 +42,11 @@ public void run() { this.watcher = watcher; filesystemMonitorFailure = Optional.empty(); - while (true) { + while (notShutdown.get()) { WatchKey key; try { key = watcher.take(); - } catch (InterruptedException e) { + } catch (InterruptedException | ClosedWatchServiceException e) { return; } @@ -66,11 +69,12 @@ public void run() { } } catch (IOException e) { filesystemMonitorFailure = Optional.of(new WatchServiceUnavailableException(e.getMessage(), - e.getLocalizedMessage(), e.getCause())); + e.getLocalizedMessage(), e.getCause())); LOGGER.warn(filesystemMonitorFailure.get().getLocalizedMessage(), e); } } + @Override public boolean isActive() { return filesystemMonitorFailure.isEmpty(); } @@ -93,4 +97,15 @@ public void addListenerForFile(Path file, FileUpdateListener listener) throws IO public void removeListener(Path path, FileUpdateListener listener) { listeners.remove(path, listener); } + + @Override + public void shutdown() { + try { + notShutdown.set(false); + watcher.close(); + } catch (IOException e) { + LOGGER.error("error closing watcher", e); + } + + } } diff --git a/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java b/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java index fcb0c74551f..e84dee6190a 100644 --- a/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java +++ b/src/main/java/org/jabref/logic/util/DelayTaskThrottler.java @@ -1,7 +1,8 @@ package org.jabref.logic.util; -import java.util.concurrent.Future; +import java.util.concurrent.Callable; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -14,6 +15,7 @@ * This class allows to throttle a list of tasks. * Use case: you have an event that occurs often, and every time you want to invoke the same task. * However, if a lot of events happen in a relatively short time span, then only one task should be invoked. + * @param * * @implNote Once {@link #schedule(Runnable)} is called, the task is delayed for a given time span. * If during this time, {@link #schedule(Runnable)} is called again, then the original task is canceled and the new one scheduled. @@ -25,7 +27,7 @@ public class DelayTaskThrottler { private final ScheduledThreadPoolExecutor executor; private final int delay; - private Future scheduledTask; + private ScheduledFuture scheduledTask; /** * @param delay delay in milliseconds @@ -37,7 +39,7 @@ public DelayTaskThrottler(int delay) { this.executor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false); } - public void schedule(Runnable command) { + public ScheduledFuture schedule(Runnable command) { if (scheduledTask != null) { scheduledTask.cancel(false); } @@ -46,6 +48,19 @@ public void schedule(Runnable command) { } catch (RejectedExecutionException e) { LOGGER.debug("Rejecting while another process is already running."); } + return scheduledTask; + } + + public ScheduledFuture scheduleTask(Callable command) { + if (scheduledTask != null) { + scheduledTask.cancel(false); + } + try { + scheduledTask = executor.schedule(command, delay, TimeUnit.MILLISECONDS); + } catch (RejectedExecutionException e) { + LOGGER.debug("Rejecting while another process is already running."); + } + return scheduledTask; } /** diff --git a/src/main/java/org/jabref/model/database/event/CoarseChangeFilter.java b/src/main/java/org/jabref/model/database/event/CoarseChangeFilter.java index 46664113397..d41e0b41b74 100644 --- a/src/main/java/org/jabref/model/database/event/CoarseChangeFilter.java +++ b/src/main/java/org/jabref/model/database/event/CoarseChangeFilter.java @@ -34,7 +34,7 @@ public CoarseChangeFilter(BibDatabaseContext bibDatabaseContext) { } @Subscribe - public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextChangedEvent event) { + public synchronized void listen(BibDatabaseContextChangedEvent event) { Runnable eventPost = () -> { // Reset total change delta totalDelta = 0; @@ -53,7 +53,7 @@ public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextCh // If editing is started boolean isNewEdit = lastFieldChanged.isEmpty(); // If other field is edited - boolean isEditOnOtherField = !lastFieldChanged.get().equals(fieldChange.getField()); + boolean isEditOnOtherField = !isNewEdit && !lastFieldChanged.get().equals(fieldChange.getField()); // Only deltas of 1 registered by fieldChange, major change means editing much content boolean isMajorChange = totalDelta >= 100; diff --git a/src/main/java/org/jabref/model/util/DummyFileUpdateMonitor.java b/src/main/java/org/jabref/model/util/DummyFileUpdateMonitor.java index 85ef84508d3..c0b77d989de 100644 --- a/src/main/java/org/jabref/model/util/DummyFileUpdateMonitor.java +++ b/src/main/java/org/jabref/model/util/DummyFileUpdateMonitor.java @@ -7,18 +7,24 @@ * Normally, you want to use {@link org.jabref.gui.util.DefaultFileUpdateMonitor} except if you don't care about updates. */ public class DummyFileUpdateMonitor implements FileUpdateMonitor { + @Override public void addListenerForFile(Path file, FileUpdateListener listener) { - + // empty } @Override public void removeListener(Path path, FileUpdateListener listener) { - + // empty } @Override public boolean isActive() { return false; } + + @Override + public void shutdown() { + // empty + } } diff --git a/src/main/java/org/jabref/model/util/FileUpdateMonitor.java b/src/main/java/org/jabref/model/util/FileUpdateMonitor.java index a34754508bf..7f9d6a32362 100644 --- a/src/main/java/org/jabref/model/util/FileUpdateMonitor.java +++ b/src/main/java/org/jabref/model/util/FileUpdateMonitor.java @@ -24,4 +24,9 @@ public interface FileUpdateMonitor { * @return true is process is running; false otherwise. */ boolean isActive(); + + /** + * stops watching for changes + */ + void shutdown(); }