From fbd177941e38433d6195d50dd1260fbd3cbd7c8c Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Tue, 2 Aug 2022 11:39:05 -0400 Subject: [PATCH] GP-0: Fix timing issues in 2 tests: BreakpointProviderTest.testActionFilter, ModelProviderTest.testActionShowPrimitives --- ...ebuggerLogicalBreakpointServicePlugin.java | 5 ++ .../DebuggerStaticMappingServicePlugin.java | 6 ++ .../DebuggerLogicalBreakpointService.java | 10 +++ .../DebuggerStaticMappingService.java | 11 +++ .../DebuggerBreakpointsProviderTest.java | 78 +++++++++++-------- .../gui/model/DebuggerModelProviderTest.java | 5 +- .../java/ghidra/async/AsyncDebouncer.java | 3 + 7 files changed, 83 insertions(+), 35 deletions(-) diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java index 1d1ed83e19..c483a91a4c 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java @@ -1060,6 +1060,11 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin changeListeners.remove(l); } + @Override + public CompletableFuture changesSettled() { + return CompletableFuture.supplyAsync(() -> null, executor); + } + protected MappedLogicalBreakpoint synthesizeLogicalBreakpoint(Program program, Address address, long length, Collection kinds) { /** diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/modules/DebuggerStaticMappingServicePlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/modules/DebuggerStaticMappingServicePlugin.java index 2c3210f8d6..135fb85288 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/modules/DebuggerStaticMappingServicePlugin.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/modules/DebuggerStaticMappingServicePlugin.java @@ -18,6 +18,7 @@ package ghidra.app.plugin.core.debug.service.modules; import java.net.URL; import java.util.*; import java.util.Map.Entry; +import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; import org.apache.commons.lang3.ArrayUtils; @@ -596,6 +597,11 @@ public class DebuggerStaticMappingServicePlugin extends Plugin changeListeners.remove(l); } + @Override + public CompletableFuture changesSettled() { + return changeDebouncer.settled(); + } + @Override public void processEvent(PluginEvent event) { if (event instanceof ProgramOpenedPluginEvent) { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java index 946f419778..054b9a7c12 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java @@ -147,6 +147,16 @@ public interface DebuggerLogicalBreakpointService { */ void removeChangeListener(LogicalBreakpointsChangeListener l); + /** + * Get a future which completes after pending changes have been processed + * + *

+ * The returned future completes after all change listeners have been invoked + * + * @return the future + */ + CompletableFuture changesSettled(); + static T programOrTrace(ProgramLocation loc, BiFunction progFunc, BiFunction traceFunc) { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerStaticMappingService.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerStaticMappingService.java index 0a012b4b19..b5a87baa19 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerStaticMappingService.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerStaticMappingService.java @@ -16,6 +16,7 @@ package ghidra.app.services; import java.util.*; +import java.util.concurrent.CompletableFuture; import com.google.common.collect.Range; @@ -400,6 +401,16 @@ public interface DebuggerStaticMappingService { */ void removeChangeListener(DebuggerStaticMappingChangeListener l); + /** + * Get a future which completes when pending changes have all settled + * + *

+ * The returned future completes after all change listeners have been invoked. + * + * @return the future + */ + CompletableFuture changesSettled(); + /** * Collect likely matches for destination programs for the given trace module * diff --git a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointsProviderTest.java b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointsProviderTest.java index edbe19c800..0a658fdc48 100644 --- a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointsProviderTest.java +++ b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointsProviderTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.*; import java.awt.event.MouseEvent; import java.util.List; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import org.junit.Before; @@ -63,12 +64,14 @@ public class DebuggerBreakpointsProviderTest extends AbstractGhidraHeadedDebugge protected DebuggerBreakpointsPlugin breakpointsPlugin; protected DebuggerBreakpointsProvider breakpointsProvider; protected DebuggerStaticMappingService mappingService; + protected DebuggerLogicalBreakpointService breakpointService; @Before public void setUpBreakpointsProviderTest() throws Exception { breakpointsPlugin = addPlugin(tool, DebuggerBreakpointsPlugin.class); breakpointsProvider = waitForComponentProvider(DebuggerBreakpointsProvider.class); mappingService = tool.getService(DebuggerStaticMappingService.class); + breakpointService = tool.getService(DebuggerLogicalBreakpointService.class); } protected void waitAndFlush(TraceRecorder recorder) throws Throwable { @@ -538,7 +541,7 @@ public class DebuggerBreakpointsProviderTest extends AbstractGhidraHeadedDebugge } @Test - public void testActionFilters() throws Exception { + public void testActionFilters() throws Throwable { createTestModel(); mb.createTestProcessesAndThreads(); @@ -561,42 +564,49 @@ public class DebuggerBreakpointsProviderTest extends AbstractGhidraHeadedDebugge addLiveBreakpoint(recorder1, 0x55550321); addLiveMemoryAndBreakpoint(mb.testProcess3, recorder3); addLiveBreakpoint(recorder3, 0x55550321); + waitRecorder(recorder1); + waitRecorder(recorder3); addStaticMemoryAndBreakpoint(); // Note, no program breakpoint for 0321... + programManager.openProgram(program); traceManager.openTrace(trace1); + CompletableFuture mappingsSettled = mappingService.changesSettled(); + CompletableFuture breakpointsSettled = breakpointService.changesSettled(); traceManager.openTrace(trace3); - // Because mapping service debounces, wait for breakpoints to be reconciled + waitForSwing(); + waitOn(mappingsSettled); + waitOn(breakpointsSettled); + waitForSwing(); + LogicalBreakpointTableModel bptModel = breakpointsProvider.breakpointTableModel; - waitForPass(() -> { - List data = copyModelData(bptModel); - assertEquals(2, data.size()); - LogicalBreakpointRow row1 = data.get(0); - LogicalBreakpointRow row2 = data.get(1); - LogicalBreakpoint lb1 = row1.getLogicalBreakpoint(); - LogicalBreakpoint lb2 = row2.getLogicalBreakpoint(); - assertEquals(program, lb1.getProgram()); - assertEquals(program, lb2.getProgram()); - assertEquals(addr(program, 0x00400123), lb1.getAddress()); - assertEquals(addr(program, 0x00400321), lb2.getAddress()); - assertEquals(Set.of(trace1, trace3), lb1.getParticipatingTraces()); - assertEquals(Set.of(trace1, trace3), lb2.getParticipatingTraces()); - // Sanity check / experiment: Equal fields, but from different traces - TraceBreakpoint bl1t1 = Unique.assertOne(lb1.getTraceBreakpoints(trace1)); - TraceBreakpoint bl1t3 = Unique.assertOne(lb1.getTraceBreakpoints(trace3)); - assertNotEquals(bl1t1, bl1t3); + List data = copyModelData(bptModel); + assertEquals(2, data.size()); + LogicalBreakpointRow row1 = data.get(0); + LogicalBreakpointRow row2 = data.get(1); + LogicalBreakpoint lb1 = row1.getLogicalBreakpoint(); + LogicalBreakpoint lb2 = row2.getLogicalBreakpoint(); + assertEquals(program, lb1.getProgram()); + assertEquals(program, lb2.getProgram()); + assertEquals(addr(program, 0x00400123), lb1.getAddress()); + assertEquals(addr(program, 0x00400321), lb2.getAddress()); + assertEquals(Set.of(trace1, trace3), lb1.getParticipatingTraces()); + assertEquals(Set.of(trace1, trace3), lb2.getParticipatingTraces()); - // OK, back to work - assertEquals(2, lb1.getTraceBreakpoints().size()); - assertEquals(2, lb2.getTraceBreakpoints().size()); - }); + // Sanity check / experiment: Equal fields, but from different traces + TraceBreakpoint bl1t1 = Unique.assertOne(lb1.getTraceBreakpoints(trace1)); + TraceBreakpoint bl1t3 = Unique.assertOne(lb1.getTraceBreakpoints(trace3)); + assertNotEquals(bl1t1, bl1t3); + + // OK, back to work + assertEquals(2, lb1.getTraceBreakpoints().size()); + assertEquals(2, lb2.getTraceBreakpoints().size()); - List breakData = copyModelData(bptModel); List filtLocs = breakpointsProvider.locationFilterPanel.getTableFilterModel().getModelData(); - for (LogicalBreakpointRow breakRow : breakData) { + for (LogicalBreakpointRow breakRow : data) { assertEquals(2, breakRow.getLocationCount()); } assertEquals(4, filtLocs.size()); @@ -605,9 +615,9 @@ public class DebuggerBreakpointsProviderTest extends AbstractGhidraHeadedDebugge performAction(breakpointsProvider.actionFilterByCurrentTrace); // No trace active, so empty :) - breakData = bptModel.getModelData(); + data = copyModelData(bptModel); filtLocs = breakpointsProvider.locationFilterPanel.getTableFilterModel().getModelData(); - for (LogicalBreakpointRow breakRow : breakData) { + for (LogicalBreakpointRow breakRow : data) { assertEquals(0, breakRow.getLocationCount()); } assertEquals(0, filtLocs.size()); @@ -615,9 +625,9 @@ public class DebuggerBreakpointsProviderTest extends AbstractGhidraHeadedDebugge traceManager.activateTrace(trace1); waitForSwing(); - breakData = bptModel.getModelData(); + data = copyModelData(bptModel); filtLocs = breakpointsProvider.locationFilterPanel.getTableFilterModel().getModelData(); - for (LogicalBreakpointRow breakRow : breakData) { + for (LogicalBreakpointRow breakRow : data) { assertEquals(1, breakRow.getLocationCount()); } assertEquals(2, filtLocs.size()); @@ -626,14 +636,14 @@ public class DebuggerBreakpointsProviderTest extends AbstractGhidraHeadedDebugge performAction(breakpointsProvider.actionFilterLocationsByBreakpoints); // No breakpoint selected, so no change, yet. - breakData = bptModel.getModelData(); + data = copyModelData(bptModel); filtLocs = breakpointsProvider.locationFilterPanel.getTableFilterModel().getModelData(); - for (LogicalBreakpointRow breakRow : breakData) { + for (LogicalBreakpointRow breakRow : data) { assertEquals(1, breakRow.getLocationCount()); } assertEquals(2, filtLocs.size()); - breakpointsProvider.setSelectedBreakpoints(Set.of(breakData.get(0).getLogicalBreakpoint())); + breakpointsProvider.setSelectedBreakpoints(Set.of(data.get(0).getLogicalBreakpoint())); waitForSwing(); filtLocs = breakpointsProvider.locationFilterPanel.getTableFilterModel().getModelData(); @@ -642,9 +652,9 @@ public class DebuggerBreakpointsProviderTest extends AbstractGhidraHeadedDebugge assertTrue(breakpointsProvider.actionFilterByCurrentTrace.isEnabled()); performAction(breakpointsProvider.actionFilterByCurrentTrace); - breakData = bptModel.getModelData(); + data = copyModelData(bptModel); filtLocs = breakpointsProvider.locationFilterPanel.getTableFilterModel().getModelData(); - for (LogicalBreakpointRow breakRow : breakData) { + for (LogicalBreakpointRow breakRow : data) { assertEquals(2, breakRow.getLocationCount()); } assertEquals(2, filtLocs.size()); diff --git a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/model/DebuggerModelProviderTest.java b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/model/DebuggerModelProviderTest.java index 52cb96ac2d..c73ca91c07 100644 --- a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/model/DebuggerModelProviderTest.java +++ b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/gui/model/DebuggerModelProviderTest.java @@ -547,16 +547,19 @@ public class DebuggerModelProviderTest extends AbstractGhidraHeadedDebuggerGUITe modelProvider.setPath(TraceObjectKeyPath.parse("Processes[0].Threads[2]")); waitForSwing(); - AbstractNode nodeThread2 = modelProvider.objectsTreePanel.getSelectedItem(); + AbstractNode nodeThread2 = + waitForValue(() -> modelProvider.objectsTreePanel.getSelectedItem()); assertEquals(1, nodeThread2.getChildren().size()); performAction(modelProvider.actionShowPrimitivesInTree, modelProvider, true); assertTrue(modelProvider.isShowPrimitivesInTree()); + nodeThread2 = waitForValue(() -> modelProvider.objectsTreePanel.getSelectedItem()); assertEquals(3, nodeThread2.getChildren().size()); assertEquals(nodeThread2, modelProvider.objectsTreePanel.getSelectedItem()); performAction(modelProvider.actionShowPrimitivesInTree, modelProvider, true); assertFalse(modelProvider.isShowPrimitivesInTree()); + nodeThread2 = waitForValue(() -> modelProvider.objectsTreePanel.getSelectedItem()); assertEquals(1, nodeThread2.getChildren().size()); assertEquals(nodeThread2, modelProvider.objectsTreePanel.getSelectedItem()); } diff --git a/Ghidra/Debug/Framework-AsyncComm/src/main/java/ghidra/async/AsyncDebouncer.java b/Ghidra/Debug/Framework-AsyncComm/src/main/java/ghidra/async/AsyncDebouncer.java index 7316218747..a10ef9ad1b 100644 --- a/Ghidra/Debug/Framework-AsyncComm/src/main/java/ghidra/async/AsyncDebouncer.java +++ b/Ghidra/Debug/Framework-AsyncComm/src/main/java/ghidra/async/AsyncDebouncer.java @@ -112,6 +112,9 @@ public class AsyncDebouncer { /** * Receive the next settled event * + *

+ * The returned future completes after all registered listeners have been invoked. + * * @return a future which completes with the value of the next settled event. */ public synchronized CompletableFuture settled() {