Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix file update monitor shutdown #6835

Merged
merged 6 commits into from
Sep 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}