Both the call to check for readiness and the actual client code will be run on the Swing
+ * thread.
+ */
+public class ExpiringSwingTimer extends GhidraSwingTimer {
+
+ private long startMs = System.currentTimeMillis();
+ private int expireMs;
+ private BooleanSupplier isReady;
+ private ExpiringTimerCallback expiringTimerCallback = new ExpiringTimerCallback();
+ private TimerCallback clientCallback;
+ private AtomicBoolean didRun = new AtomicBoolean();
+
+ /**
+ * Runs the given client runnable when the given condition returns true
+ *
+ * @param isReady true if the code should be run
+ * @param expireMs the amount of time past which the code will not be run
+ * @param runnable the code to run
+ * @return the timer object that is running, which will execute the given code when ready
+ */
+ public static ExpiringSwingTimer runWhen(BooleanSupplier isReady,
+ int expireMs,
+ Runnable runnable) {
+
+ // Note: we could let the client specify the period, but that would add an extra argument
+ // to this method. For now, just use something reasonable.
+ int delay = 250;
+ ExpiringSwingTimer timer =
+ new ExpiringSwingTimer(delay, expireMs, isReady, runnable);
+ timer.setRepeats(true);
+ timer.start();
+ return timer;
+ }
+
+ /**
+ * Constructor
+ *
+ * @param delay the delay between calls to check isReady
+ * @param isReady true if the code should be run
+ * @param expireMs the amount of time past which the code will not be run
+ * @param runnable the code to run
+ */
+ public ExpiringSwingTimer(int delay, int expireMs, BooleanSupplier isReady,
+ Runnable runnable) {
+ super(delay, null);
+ this.expireMs = expireMs;
+ this.isReady = isReady;
+ this.clientCallback = () -> runnable.run();
+ super.setTimerCallback(expiringTimerCallback);
+ }
+
+ /**
+ * Returns true if the client runnable was run
+ * @return true if the client runnable was run
+ */
+ public boolean didRun() {
+ return didRun.get();
+ }
+
+ @Override
+ public void setTimerCallback(TimerCallback callback) {
+ // overridden to ensure clients cannot overwrite out wrapping callback
+ this.clientCallback = Objects.requireNonNull(callback);
+ }
+
+ private class ExpiringTimerCallback implements TimerCallback {
+
+ @Override
+ public void timerFired() {
+
+ long now = System.currentTimeMillis();
+ int passed = (int) (now - startMs);
+ if (passed > expireMs) {
+ timer.stop();
+ return;
+ }
+
+ if (!isReady.getAsBoolean()) {
+ return;
+ }
+
+ clientCallback.timerFired();
+ stop();
+ didRun.set(true);
+ }
+ }
+}
diff --git a/Ghidra/Framework/Generic/src/main/java/generic/timer/GhidraSwingTimer.java b/Ghidra/Framework/Generic/src/main/java/generic/timer/GhidraSwingTimer.java
index b3cb1b7053..f66fcb68eb 100644
--- a/Ghidra/Framework/Generic/src/main/java/generic/timer/GhidraSwingTimer.java
+++ b/Ghidra/Framework/Generic/src/main/java/generic/timer/GhidraSwingTimer.java
@@ -1,6 +1,5 @@
/* ###
* IP: GHIDRA
- * REVIEWED: YES
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -22,67 +21,78 @@ import java.awt.event.ActionListener;
import javax.swing.Timer;
public class GhidraSwingTimer implements GhidraTimer, ActionListener {
- private Timer timer;
+
+ Timer timer;
private TimerCallback callback;
-
+
public GhidraSwingTimer() {
this(100, null);
}
-
+
public GhidraSwingTimer(int delay, TimerCallback callback) {
- this(delay,delay,callback);
+ this(delay, delay, callback);
}
-
+
public GhidraSwingTimer(int initialDelay, int delay, TimerCallback callback) {
this.callback = callback;
timer = new Timer(delay, this);
timer.setInitialDelay(delay);
}
-
+
+ @Override
public void actionPerformed(ActionEvent e) {
if (callback != null) {
callback.timerFired();
}
}
-
+
+ @Override
public int getDelay() {
return timer.getDelay();
}
+ @Override
public int getInitialDelay() {
return timer.getInitialDelay();
}
+ @Override
public boolean isRepeats() {
return timer.isRepeats();
}
+ @Override
public void setDelay(int delay) {
timer.setDelay(delay);
}
+ @Override
public void setInitialDelay(int initialDelay) {
timer.setInitialDelay(initialDelay);
}
+ @Override
public void setRepeats(boolean repeats) {
timer.setRepeats(repeats);
}
+ @Override
public void setTimerCallback(TimerCallback callback) {
this.callback = callback;
}
+ @Override
public void start() {
timer.start();
}
+ @Override
public void stop() {
timer.stop();
}
+ @Override
public boolean isRunning() {
return timer.isRunning();
}
-
}
diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/SwingUpdateManager.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/SwingUpdateManager.java
index c7ad18b192..030ff59154 100644
--- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/SwingUpdateManager.java
+++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/task/SwingUpdateManager.java
@@ -15,8 +15,6 @@
*/
package ghidra.util.task;
-import java.util.concurrent.atomic.AtomicInteger;
-
import javax.swing.Timer;
import ghidra.util.Msg;
@@ -50,13 +48,8 @@ import utilities.util.reflection.ReflectionUtilities;
*
*
This class is safe to use in a multi-threaded environment. State variables are guarded * via synchronization on this object. The Swing thread is used to perform updates, which - * guarantees that only one update will happen at a time. There is one state variable, - * the {@link #workCount}, that is changed both in the synchronized blocks and the Swing thread - * which is an atomic variable. This variable must be updated/incremented when the - * synchronized variables are cleared to prevent {@link #isBusy()} from returning false when - * there is a gap between 'work posted' and 'work execution'. + * guarantees that only one update will happen at a time. */ - public class SwingUpdateManager { private static final long NONE = 0; public static final int DEFAULT_MAX_DELAY = 30000; @@ -78,11 +71,13 @@ public class SwingUpdateManager { private long bufferingStartTime; private boolean disposed = false; - // this is the number of times we will be calling work - private AtomicInteger workCount = new AtomicInteger(); + // This is true when work has begun and is not finished. This is only mutated on the + // Swing thread, but is read by other threads. + private boolean isWorking; /** - * Constructs a new SwingUpdateManager. + * Constructs a new SwingUpdateManager with default values for min and max delay. See + * {@link #DEFAULT_MIN_DELAY} and {@value #DEFAULT_MAX_DELAY}. * * @param r the runnable that performs the client work. */ @@ -171,7 +166,7 @@ public class SwingUpdateManager { } requestTime = System.currentTimeMillis(); - bufferingStartTime = bufferingStartTime == 0 ? requestTime : bufferingStartTime; + bufferingStartTime = bufferingStartTime == NONE ? requestTime : bufferingStartTime; scheduleCheckForWork(); } @@ -219,15 +214,15 @@ public class SwingUpdateManager { } /** - * Returns true if any work is being performed or if there is buffered work. - * @return true if any work is being performed or if there is buffered work. + * Returns true if any work is being performed or if there is buffered work + * @return true if any work is being performed or if there is buffered work */ public synchronized boolean isBusy() { if (disposed) { return false; } - return requestTime != NONE || workCount.get() != 0; + return requestTime != NONE || isWorking; } public synchronized void dispose() { @@ -253,21 +248,25 @@ public class SwingUpdateManager { "\tname: " + name + "\n" + "\tcreator: " + inceptionInformation + " ("+System.identityHashCode(this)+")\n" + "\trequest time: "+requestTime + "\n" + - "\twork count: " + workCount.get() + "\n" + + "\twork count: " + isWorking + "\n" + "}"; //@formatter:on } + // note: this is called on the Swing thread private void checkForWork() { + if (shouldDoWork()) { doWork(); } } + // note: this is called on the Swing thread private synchronized boolean shouldDoWork() { - // If no pending request, exit without restarting timer. + // If no pending request, exit without restarting timer if (requestTime == NONE) { + bufferingStartTime = NONE; // The timer has fired and there is no pending work return false; } @@ -275,7 +274,7 @@ public class SwingUpdateManager { if (isTimeToWork(now)) { bufferingStartTime = now; requestTime = NONE; - workCount.incrementAndGet(); + isWorking = true; return true; } @@ -304,6 +303,7 @@ public class SwingUpdateManager { return false; } + // note: this is called on the Swing thread private void doWork() { try { clientRunnable.run(); @@ -313,9 +313,11 @@ public class SwingUpdateManager { Msg.showError(this, null, "Unexpected Exception", "Unexpected exception in Swing Update Manager", t); } - finally { - workCount.decrementAndGet(); - } + + isWorking = false; + + // we need to clear the buffering flag after the minDelay has passed, so start the timer + scheduleCheckForWork(); } //================================================================================================== diff --git a/Ghidra/Framework/Generic/src/test/java/generic/timer/ExpiringSwingTimerTest.java b/Ghidra/Framework/Generic/src/test/java/generic/timer/ExpiringSwingTimerTest.java new file mode 100644 index 0000000000..03eeed23c6 --- /dev/null +++ b/Ghidra/Framework/Generic/src/test/java/generic/timer/ExpiringSwingTimerTest.java @@ -0,0 +1,65 @@ +/* ### + * IP: GHIDRA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package generic.timer; + +import static org.junit.Assert.*; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BooleanSupplier; + +import org.junit.Test; + +import generic.test.AbstractGenericTest; + +public class ExpiringSwingTimerTest extends AbstractGenericTest { + + @Test + public void testRunWhenReady() { + + int waitCount = 2; + AtomicInteger counter = new AtomicInteger(); + BooleanSupplier isReady = () -> { + return counter.incrementAndGet() > waitCount; + }; + + AtomicInteger runCount = new AtomicInteger(); + Runnable r = () -> { + runCount.incrementAndGet(); + }; + ExpiringSwingTimer.runWhen(isReady, 10000, r); + + waitFor(() -> runCount.get() > 0); + assertTrue("Timer did not wait for the condition to be true", counter.get() > waitCount); + assertEquals("Client code was run more than once", 1, runCount.get()); + } + + @Test + public void testRunWhenReady_Timeout() { + + BooleanSupplier isReady = () -> { + return false; + }; + + AtomicBoolean didRun = new AtomicBoolean(); + Runnable r = () -> didRun.set(true); + ExpiringSwingTimer timer = ExpiringSwingTimer.runWhen(isReady, 500, r); + + waitFor(() -> !timer.isRunning()); + + assertFalse(didRun.get()); + } +} diff --git a/Ghidra/Framework/Generic/src/test/java/ghidra/util/task/SwingUpdateManagerTest.java b/Ghidra/Framework/Generic/src/test/java/ghidra/util/task/SwingUpdateManagerTest.java index 66af3539fc..d1ea338841 100644 --- a/Ghidra/Framework/Generic/src/test/java/ghidra/util/task/SwingUpdateManagerTest.java +++ b/Ghidra/Framework/Generic/src/test/java/ghidra/util/task/SwingUpdateManagerTest.java @@ -37,6 +37,11 @@ public class SwingUpdateManagerTest extends AbstractGenericTest { @Before public void setUp() throws Exception { manager = createUpdateManager(MIN_DELAY, MAX_DELAY); + + // must turn this on to get the expected results, as in headless mode the update manager + // will run it's Swing work immediately on the test thread, which is not true to the + // default behavior + System.setProperty(SystemUtilities.HEADLESS_PROPERTY, Boolean.FALSE.toString()); } @Test @@ -160,20 +165,16 @@ public class SwingUpdateManagerTest extends AbstractGenericTest { // before the update runnable is called. // - // must turn this on to get the expected results, as in headless mode the update manager - // will run it's Swing work immediately - System.setProperty(SystemUtilities.HEADLESS_PROPERTY, Boolean.FALSE.toString()); - - final CountDownLatch startGate = new CountDownLatch(1); - final CountDownLatch endGate = new CountDownLatch(1); - final AtomicBoolean exception = new AtomicBoolean(); + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch endLatch = new CountDownLatch(1); + AtomicBoolean exception = new AtomicBoolean(); runSwing(new Runnable() { @Override public void run() { - startGate.countDown(); + startLatch.countDown(); try { - endGate.await(10, TimeUnit.SECONDS); + endLatch.await(10, TimeUnit.SECONDS); } catch (InterruptedException e) { exception.set(true); @@ -181,13 +182,13 @@ public class SwingUpdateManagerTest extends AbstractGenericTest { } }, false); - // This will cause the swing thread to block until will countdown the endGate latch - startGate.await(10, TimeUnit.SECONDS); + // This will cause the swing thread to block until we countdown the end latch + startLatch.await(10, TimeUnit.SECONDS); manager.update(); assertTrue("Manager not busy after requesting an update", manager.isBusy()); - endGate.countDown(); + endLatch.countDown(); waitForManager(); assertTrue("Manager still busy after waiting for update", !manager.isBusy()); @@ -195,9 +196,61 @@ public class SwingUpdateManagerTest extends AbstractGenericTest { assertFalse("Interrupted waiting for CountDowLatch", exception.get()); } + @Test + public void testCallToUpdateWhileAnUpdateIsWorking() throws Exception { + + // + // Test that an update call from a non-swing thread will still get processed if the + // manager is actively processing an update on the swing thread. + // + + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch endLatch = new CountDownLatch(1); + AtomicBoolean exception = new AtomicBoolean(); + + Runnable r = () -> { + runnableCalled++; + + startLatch.countDown(); + try { + endLatch.await(10, TimeUnit.SECONDS); + } + catch (InterruptedException e) { + exception.set(true); + } + }; + + // start the update manager and have it wait for us + manager = new SwingUpdateManager(MIN_DELAY, MAX_DELAY, r); + manager.update(); + + // have the swing thread block until we countdown the end latch + startLatch.await(10, TimeUnit.SECONDS); + + // post the second update request now that the manager is actively processing + manager.update(); + + // let the update manager finish; verify 2 work items total + endLatch.countDown(); + waitForManager(); + assertEquals("Expected exactly 2 callbacks", 2, runnableCalled); + } + //============================================================================================== // Private Methods //============================================================================================== + private void waitForManager() { + + // let all swing updates finish, which may trigger the update manager + waitForSwing(); + + while (manager.isBusy()) { + sleep(DEFAULT_WAIT_DELAY); + } + + // let any resulting swing events finish + waitForSwing(); + } private SwingUpdateManager createUpdateManager(int min, int max) { return new SwingUpdateManager(min, max, new Runnable() { @@ -209,19 +262,4 @@ public class SwingUpdateManagerTest extends AbstractGenericTest { } }); } - - private void waitForManager() { - - // let all swing updates finish, which may trigger the update manager - waitForPostedSwingRunnables(); - - while (manager.isBusy()) { - sleep(DEFAULT_WAIT_DELAY); - } - - // let any resulting swing events finish - waitForPostedSwingRunnables(); - - } - }