diff --git a/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/CachingSwingWorker.java b/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/CachingSwingWorker.java index 8b81341928..c6914d9138 100644 --- a/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/CachingSwingWorker.java +++ b/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/CachingSwingWorker.java @@ -18,8 +18,7 @@ package ghidra.util.task; import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.ExecutionException; +import java.util.concurrent.*; import javax.swing.SwingUtilities; import javax.swing.SwingWorker; @@ -62,7 +61,7 @@ public abstract class CachingSwingWorker implements CachingLoader { private int taskDialogDelay = 500; private boolean hasProgress = true; private T cachedValue; - private SwingWorker worker; + private SwingWorkerImpl worker; private WorkerTaskMonitor taskMonitor = new WorkerTaskMonitor(); /** @@ -86,7 +85,7 @@ public abstract class CachingSwingWorker implements CachingLoader { /** * Subclasses must implement this method to create the object being managed/cached. * @param monitor A task monitor that can be used to provide progress information to - * a progress dialog is it is being shown. Implementers should also check the monitor + * a progress dialog is it is being shown. Implementers should also check the monitor * periodically to check for cancelled. If cancelled, this method should not throw a * cancelled exception but instead either return null or a partial result. (For example, * if the object is a list being generated by a search, then it might make sense to return @@ -97,7 +96,7 @@ public abstract class CachingSwingWorker implements CachingLoader { /** * Returns the object that this class is managing/caching. It will return the object if it is - * already created or it will block until the object can be created. If called from the Swing + * already created or it will block until the object can be created. If called from the Swing * thread, it will also launch a modal progress dialog while waiting for the object to be * created. * @@ -113,7 +112,7 @@ public abstract class CachingSwingWorker implements CachingLoader { } addMonitor(monitor); - SwingWorker swingWorker = getWorker(); + SwingWorkerImpl swingWorker = getWorker(); if (SwingUtilities.isEventDispatchThread()) { blockSwingWithProgressDialog(swingWorker); } @@ -175,7 +174,7 @@ public abstract class CachingSwingWorker implements CachingLoader { } } - private void blockSwingWithProgressDialog(final SwingWorker localWorker) { + private void blockSwingWithProgressDialog(SwingWorkerImpl localWorker) { if (!localWorker.isDone()) { TaskDialog dialog = new SwingWorkerTaskDialog(name, hasProgress, localWorker); @@ -185,7 +184,7 @@ public abstract class CachingSwingWorker implements CachingLoader { } } - private synchronized SwingWorker getWorker() { + private synchronized SwingWorkerImpl getWorker() { if (worker == null) { worker = new SwingWorkerImpl(); worker.execute(); @@ -220,19 +219,19 @@ public abstract class CachingSwingWorker implements CachingLoader { //================================================================================================== // Inner Classes -//================================================================================================== +//================================================================================================== private class SwingWorkerTaskDialog extends TaskDialog { - private SwingWorker taskWorker; + private SwingWorkerImpl taskWorker; - SwingWorkerTaskDialog(String title, boolean showProgress, SwingWorker worker) { - super(title, true, true, showProgress); + SwingWorkerTaskDialog(String title, boolean showProgress, SwingWorkerImpl worker) { + super(title, true, true, showProgress, worker.getFinishedLatch()); this.taskWorker = worker; } @Override - // Overridden to allow us to check for the worker's completion. If we don't do + // Overridden to allow us to check for the worker's completion. If we don't do // this, then there is no way for the task dialog to know when the work is finished. // The result of this is that the task dialog gets shown every time, regardless of // whether the worker finished before the 'grace' period. @@ -242,9 +241,20 @@ public abstract class CachingSwingWorker implements CachingLoader { } private class SwingWorkerImpl extends SwingWorker { + + // We update this latch *in the background thread* to notify clients in other threads that + // the work has been finished. This prevents the Swing thread from blocking while waiting + // for the callbacks that happen on the Swing thread, such as done(). + private CountDownLatch finished = new CountDownLatch(1); + + CountDownLatch getFinishedLatch() { + return finished; + } + @Override protected T doInBackground() throws Exception { T result = runInBackground(taskMonitor); + finished.countDown(); return result; } @@ -263,7 +273,7 @@ public abstract class CachingSwingWorker implements CachingLoader { } private class WorkerTaskMonitor extends TaskMonitorAdapter { - private List monitors = new CopyOnWriteArrayList(); + private List monitors = new CopyOnWriteArrayList<>(); private int min = 0; private int max = 0; private int progress = 0; diff --git a/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskDialog.java b/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskDialog.java index 2195f56b29..2d89997e64 100644 --- a/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskDialog.java +++ b/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskDialog.java @@ -35,7 +35,9 @@ import ghidra.util.timer.GTimerMonitor; /** * Dialog that is displayed to show activity for a Task that is running outside of the - * Swing Thread. + * Swing Thread. This dialog uses a delay before showing in order to give the background task + * thread a chance to finish. This prevents a flashing dialog for tasks that finish before the + * delay time period. * *

Implementation note: * if this class is constructed with a {@code hasProgress} value of {@code false}, @@ -47,6 +49,16 @@ import ghidra.util.timer.GTimerMonitor; * *

This dialog can be toggled between indeterminate mode and progress mode via calls to * {@link #setIndeterminate(boolean)}. + * + *

API Usage Note: If this dialog is used outside of the task API, then the client must + * be sure to call {@link #taskProcessed()} from the background thread performing the work. + * Otherwise, this dialog will always wait for the {@code delay} amount of time for the background + * thread to finish. This happens since the default completed notification mechanism is performed + * on the Swing thread. If a client has triggered blocking on the Swing thread, then the + * notification on the Swing thread must wait, causing the full delay to take place. Calling + * {@link #taskProcessed()} from the background thread allows the dialog to get notified before the + * {@code delay} period has expired. The blocking issue only exists with a non-0 {@code delay} + * value. */ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { @@ -66,23 +78,23 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { * -calls iternalCancel() * -triggers taskProcessed() * -calls closeDialog runnable - * + * * Cancel Button Pressed: * -(same as Dialog Close Button Pressed) * * Task Monitor Stop Button Pressed: * -triggers taskProcessed() * -calls closeDialog runnable - * + * * Public API dispose() is Called: * -calls iternalCancel() * -triggers taskProcessed() * -calls closeDialog runnable - * + * * Task Monitor Cancelled by API: * -triggers taskProcessed() * -calls closeDialog runnable - * + * * Task Finishes Normally: * -triggers taskProcessed() * -calls closeDialog runnable @@ -100,7 +112,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { }; private GTimerMonitor showTimer = GTimerMonitor.DUMMY; - private CountDownLatch finished = new CountDownLatch(1); + private CountDownLatch finished; private boolean supportsProgress; private JPanel mainPanel; @@ -121,12 +133,11 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { * @param centerOnComp component to be centered over when shown, otherwise center over parent. * If both centerOnComp and parent are null, dialog will be centered on screen. * @param task the Task that this dialog will be associated with - * @param finished overrides the latch used by this dialog to know when the task is finished + * @param finished the finished latch used by the background thread to notify of completion */ TaskDialog(Component centerOnComp, Task task, CountDownLatch finished) { this(centerOnComp, task.getTaskTitle(), task.isModal(), task.canCancel(), - task.hasProgress()); - this.finished = finished; + task.hasProgress(), finished); } /** @@ -147,7 +158,21 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { * @param hasProgress true if the dialog should show a progress bar */ public TaskDialog(String title, boolean canCancel, boolean isModal, boolean hasProgress) { - this(null, title, isModal, canCancel, hasProgress); + this(title, isModal, canCancel, hasProgress, new CountDownLatch(1)); + } + + /** + * Constructor + * + * @param title title for the dialog + * @param canCancel true if the task can be canceled + * @param isModal true if the dialog should be modal + * @param hasProgress true if the dialog should show a progress bar + * @param finished the finished latch used by the background thread to notify of completion + */ + public TaskDialog(String title, boolean canCancel, boolean isModal, boolean hasProgress, + CountDownLatch finished) { + this(null, title, isModal, canCancel, hasProgress, finished); } /** @@ -159,12 +184,14 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { * @param isModal true if the dialog should be modal * @param canCancel true if the task can be canceled * @param hasProgress true if the dialog should show a progress bar + * @param finished the finished latch used by the background thread to notify of completion */ private TaskDialog(Component centerOnComp, String title, boolean isModal, boolean canCancel, - boolean hasProgress) { + boolean hasProgress, CountDownLatch finished) { super(title, isModal, true, canCancel, true); this.centerOnComponent = centerOnComp; this.supportsProgress = hasProgress; + this.finished = finished; setup(canCancel); } @@ -362,6 +389,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { /** * Cancels the task and closes this dialog */ + @Override public void dispose() { internalCancel(); } @@ -409,7 +437,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { setIndeterminate(false); } - // Note: it is not clear why we only wish to show progress if the monitor is not + // Note: it is not clear why we only wish to show progress if the monitor is not // visible. This seems wrong. If someone knows, please update this code. //if (!monitorComponent.isShowing()) { installProgressMonitor(); @@ -431,7 +459,7 @@ public class TaskDialog extends DialogComponentProvider implements TaskMonitor { supportsProgress = !indeterminate; monitorComponent.setIndeterminate(indeterminate); - // Assumption: if the client calls this method to show progress, then we should honor + // Assumption: if the client calls this method to show progress, then we should honor // that request. If we find that nested monitor usage causes dialogs to incorrectly // toggle monitors, then we need to update those clients to use a wrapping style // monitor that prevents the behavior.