Skip to content

Commit

Permalink
Fix file update monitor shutdown (#6835)
Browse files Browse the repository at this point in the history
  • Loading branch information
Siedlerchr committed Sep 2, 2020
1 parent deaa968 commit 5c9edc9
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/main/java/org/jabref/Globals.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public static FileUpdateMonitor getFileUpdateMonitor() {

public static void shutdownThreadPools() {
TASK_EXECUTOR.shutdown();
fileUpdateMonitor.shutdown();
JabRefExecutorService.INSTANCE.shutdownEverything();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class DatabaseChangeMonitor implements FileUpdateListener {
private final BibDatabaseContext database;
private final FileUpdateMonitor fileMonitor;
private final List<DatabaseChangeListener> listeners;
private TaskExecutor taskExecutor;
private final TaskExecutor taskExecutor;

public DatabaseChangeMonitor(BibDatabaseContext database, FileUpdateMonitor fileMonitor, TaskExecutor taskExecutor) {
this.database = database;
Expand Down
21 changes: 18 additions & 3 deletions src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
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;
import java.nio.file.WatchEvent;
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;
Expand All @@ -31,6 +33,7 @@ public class DefaultFileUpdateMonitor implements Runnable, FileUpdateMonitor {

private final Multimap<Path, FileUpdateListener> listeners = ArrayListMultimap.create(20, 4);
private WatchService watcher;
private final AtomicBoolean notShutdown = new AtomicBoolean(true);
private Optional<JabRefException> filesystemMonitorFailure;

@Override
Expand All @@ -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;
}

Expand All @@ -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();
}
Expand All @@ -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);
}

}
}
21 changes: 18 additions & 3 deletions src/main/java/org/jabref/logic/util/DelayTaskThrottler.java
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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 <T>
*
* @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.
Expand All @@ -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
Expand All @@ -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);
}
Expand All @@ -46,6 +48,19 @@ public void schedule(Runnable command) {
} catch (RejectedExecutionException e) {
LOGGER.debug("Rejecting while another process is already running.");
}
return scheduledTask;
}

public <T> 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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
10 changes: 8 additions & 2 deletions src/main/java/org/jabref/model/util/DummyFileUpdateMonitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
5 changes: 5 additions & 0 deletions src/main/java/org/jabref/model/util/FileUpdateMonitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,9 @@ public interface FileUpdateMonitor {
* @return true is process is running; false otherwise.
*/
boolean isActive();

/**
* stops watching for changes
*/
void shutdown();
}

0 comments on commit 5c9edc9

Please sign in to comment.