GP-0: Fixed DefaultDebuggerObjectModelTest

This commit is contained in:
Dan 2021-04-08 13:31:51 -04:00
parent 424cbf0a5e
commit a0163ff2f6
4 changed files with 116 additions and 65 deletions

View file

@ -471,6 +471,7 @@ public interface DebuggerObjectModel {
/** /**
* @see #fetchModelObject(List) * @see #fetchModelObject(List)
*/ */
@Deprecated
public default CompletableFuture<? extends TargetObject> fetchModelObject(String... path) { public default CompletableFuture<? extends TargetObject> fetchModelObject(String... path) {
return fetchModelObject(List.of(path)); return fetchModelObject(List.of(path));
} }

View file

@ -31,6 +31,7 @@ import ghidra.util.datastruct.ListenerSet;
public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectModel { public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectModel {
public final Object lock = new Object(); public final Object lock = new Object();
public final Object cbLock = new Object();
protected final ExecutorService clientExecutor = protected final ExecutorService clientExecutor =
Executors.newSingleThreadExecutor(new BasicThreadFactory.Builder() Executors.newSingleThreadExecutor(new BasicThreadFactory.Builder()
.namingPattern(getClass().getSimpleName() + "-thread-%d") .namingPattern(getClass().getSimpleName() + "-thread-%d")
@ -40,10 +41,12 @@ public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectMo
protected SpiTargetObject root; protected SpiTargetObject root;
protected boolean rootAdded; protected boolean rootAdded;
protected boolean cbRootAdded;
protected CompletableFuture<SpiTargetObject> completedRoot = new CompletableFuture<>(); protected CompletableFuture<SpiTargetObject> completedRoot = new CompletableFuture<>();
// Remember the order of creation events // Remember the order of creation events
protected final Map<List<String>, SpiTargetObject> creationLog = new LinkedHashMap<>(); protected final Map<List<String>, SpiTargetObject> creationLog = new LinkedHashMap<>();
protected final Map<List<String>, SpiTargetObject> cbCreationLog = new LinkedHashMap<>();
protected void objectCreated(SpiTargetObject object) { protected void objectCreated(SpiTargetObject object) {
synchronized (lock) { synchronized (lock) {
@ -55,6 +58,14 @@ public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectMo
this.root = object; this.root = object;
} }
} }
CompletableFuture.runAsync(() -> {
synchronized (cbLock) {
cbCreationLog.put(object.getPath(), object);
}
}, clientExecutor).exceptionally(ex -> {
Msg.error(this, "Error updating objectCreated before callback");
return null;
});
} }
protected void objectInvalidated(TargetObject object) { protected void objectInvalidated(TargetObject object) {
@ -65,11 +76,20 @@ public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectMo
assert root == this.root; assert root == this.root;
synchronized (lock) { synchronized (lock) {
rootAdded = true; rootAdded = true;
root.getSchema()
.validateTypeAndInterfaces(root, null, null, root.enforcesStrictSchema());
this.completedRoot.completeAsync(() -> root, clientExecutor);
listeners.fire.rootAdded(root);
} }
root.getSchema()
.validateTypeAndInterfaces(root, null, null, root.enforcesStrictSchema());
CompletableFuture.runAsync(() -> {
synchronized (cbLock) {
cbRootAdded = true;
}
completedRoot.complete(root);
}, clientExecutor).exceptionally(ex -> {
Msg.error(this, "Error updating rootAdded before callback");
return null;
});
this.completedRoot.completeAsync(() -> root, clientExecutor);
listeners.fire.rootAdded(root);
} }
@Override @Override
@ -84,27 +104,25 @@ public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectMo
} }
} }
protected void onClientExecutor(DebuggerModelListener listener, Runnable r) { protected void replayed(DebuggerModelListener listener, Runnable r) {
CompletableFuture.runAsync(r, clientExecutor).exceptionally(t -> { try {
r.run();
}
catch (Throwable t) {
Msg.error(this, "Listener " + listener + " caused unexpected exception", t); Msg.error(this, "Listener " + listener + " caused unexpected exception", t);
return null; }
});
} }
protected void replayTreeEvents(DebuggerModelListener listener) { protected void replayTreeEvents(DebuggerModelListener listener) {
if (root == null) { for (SpiTargetObject object : cbCreationLog.values()) {
assert creationLog.isEmpty(); replayed(listener, () -> listener.created(object));
return;
}
for (SpiTargetObject object : creationLog.values()) {
onClientExecutor(listener, () -> listener.created(object));
} }
Set<SpiTargetObject> visited = new HashSet<>(); Set<SpiTargetObject> visited = new HashSet<>();
for (SpiTargetObject object : creationLog.values()) { for (SpiTargetObject object : cbCreationLog.values()) {
replayAddEvents(listener, object, visited); replayAddEvents(listener, object, visited);
} }
if (rootAdded) { if (cbRootAdded) {
onClientExecutor(listener, () -> listener.rootAdded(root)); replayed(listener, () -> listener.rootAdded(root));
} }
} }
@ -113,41 +131,44 @@ public abstract class AbstractDebuggerObjectModel implements SpiDebuggerObjectMo
if (!visited.add(object)) { if (!visited.add(object)) {
return; return;
} }
Map<String, ?> cachedAttributes = object.getCachedAttributes(); Map<String, ?> cbAttributes = object.getCallbackAttributes();
for (Object val : cachedAttributes.values()) { for (Object val : cbAttributes.values()) {
if (!(val instanceof TargetObject)) { if (!(val instanceof TargetObject)) {
continue; continue;
} }
assert val instanceof SpiTargetObject; assert val instanceof SpiTargetObject;
replayAddEvents(listener, (SpiTargetObject) val, visited); replayAddEvents(listener, (SpiTargetObject) val, visited);
} }
if (!cachedAttributes.isEmpty()) { if (!cbAttributes.isEmpty()) {
onClientExecutor(listener, replayed(listener,
() -> listener.attributesChanged(object, List.of(), cachedAttributes)); () -> listener.attributesChanged(object, List.of(), cbAttributes));
} }
Map<String, ? extends TargetObject> cachedElements = object.getCachedElements(); Map<String, ? extends TargetObject> cbElements = object.getCallbackElements();
for (TargetObject elem : cachedElements.values()) { for (TargetObject elem : cbElements.values()) {
assert elem instanceof SpiTargetObject; assert elem instanceof SpiTargetObject;
replayAddEvents(listener, (SpiTargetObject) elem, visited); replayAddEvents(listener, (SpiTargetObject) elem, visited);
} }
if (!cachedElements.isEmpty()) { if (!cbElements.isEmpty()) {
onClientExecutor(listener, replayed(listener,
() -> listener.elementsChanged(object, List.of(), cachedElements)); () -> listener.elementsChanged(object, List.of(), cbElements));
} }
} }
@Override @Override
public void addModelListener(DebuggerModelListener listener, boolean replay) { public void addModelListener(DebuggerModelListener listener, boolean replay) {
try { if (replay) {
synchronized (lock) { synchronized (cbLock) {
if (replay) { CompletableFuture.runAsync(() -> {
replayTreeEvents(listener); replayTreeEvents(listener);
} listeners.add(listener);
listeners.add(listener); }, clientExecutor).exceptionally(ex -> {
listener.catastrophic(ex);
return null;
});
} }
} }
catch (Throwable ex) { else {
listener.catastrophic(ex); listeners.add(listener);
} }
} }

View file

@ -310,7 +310,9 @@ public class DefaultTargetObject<E extends TargetObject, P extends TargetObject>
private void updateCallbackElements(Delta<E, E> delta) { private void updateCallbackElements(Delta<E, E> delta) {
CompletableFuture.runAsync(() -> { CompletableFuture.runAsync(() -> {
delta.apply(this.cbElements, Delta.SAME); synchronized (model.cbLock) {
delta.apply(this.cbElements, Delta.SAME);
}
}, model.clientExecutor).exceptionally(ex -> { }, model.clientExecutor).exceptionally(ex -> {
Msg.error(this, "Error updating elements before callback"); Msg.error(this, "Error updating elements before callback");
return null; return null;
@ -489,7 +491,9 @@ public class DefaultTargetObject<E extends TargetObject, P extends TargetObject>
private void updateCallbackAttributes(Delta<Object, ?> delta) { private void updateCallbackAttributes(Delta<Object, ?> delta) {
CompletableFuture.runAsync(() -> { CompletableFuture.runAsync(() -> {
delta.apply(this.cbAttributes, Delta.EQUAL); synchronized (model.cbLock) {
delta.apply(this.cbAttributes, Delta.EQUAL);
}
}, model.clientExecutor).exceptionally(ex -> { }, model.clientExecutor).exceptionally(ex -> {
Msg.error(this, "Error updating elements before callback"); Msg.error(this, "Error updating elements before callback");
return null; return null;

View file

@ -15,7 +15,7 @@
*/ */
package ghidra.dbg.agent; package ghidra.dbg.agent;
import static ghidra.lifecycle.Unfinished.*; import static ghidra.lifecycle.Unfinished.TODO;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import java.util.*; import java.util.*;
@ -34,6 +34,7 @@ import ghidra.dbg.testutil.*;
import ghidra.dbg.testutil.AttributesChangedListener.AttributesChangedInvocation; import ghidra.dbg.testutil.AttributesChangedListener.AttributesChangedInvocation;
import ghidra.dbg.testutil.ElementsChangedListener.ElementsChangedInvocation; import ghidra.dbg.testutil.ElementsChangedListener.ElementsChangedInvocation;
import ghidra.dbg.testutil.InvalidatedListener.InvalidatedInvocation; import ghidra.dbg.testutil.InvalidatedListener.InvalidatedInvocation;
import ghidra.dbg.util.PathUtils;
import ghidra.program.model.address.AddressFactory; import ghidra.program.model.address.AddressFactory;
import ghidra.program.model.address.AddressSpace; import ghidra.program.model.address.AddressSpace;
@ -116,7 +117,7 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
@Test @Test
public void testGetModelObjectLen0() throws Throwable { public void testGetModelObjectLen0() throws Throwable {
assertEquals(model.root, waitOn(model.fetchModelObject())); assertEquals(model.root, waitOn(model.fetchModelValue()));
} }
@Test @Test
@ -124,7 +125,7 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
FakeTargetObject a = new FakeTargetObject(model, model.root, "A"); FakeTargetObject a = new FakeTargetObject(model, model.root, "A");
model.root.changeAttributes(List.of(), Map.of("A", a), "Test"); model.root.changeAttributes(List.of(), Map.of("A", a), "Test");
assertEquals(a, waitOn(model.fetchModelObject("A"))); assertEquals(a, waitOn(model.fetchModelValue("A")));
} }
@Test @Test
@ -132,7 +133,7 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
FakeTargetObject a = new FakeTargetObject(model, model.root, "A"); FakeTargetObject a = new FakeTargetObject(model, model.root, "A");
model.root.changeAttributes(List.of(), Map.of("A", a), "Test"); model.root.changeAttributes(List.of(), Map.of("A", a), "Test");
assertEquals(null, waitOn(model.fetchModelObject("NoA"))); assertEquals(null, waitOn(model.fetchModelValue("NoA")));
} }
@Test @Test
@ -143,7 +144,7 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
FakeTargetObject b = new FakeTargetObject(model, a, "[B]"); FakeTargetObject b = new FakeTargetObject(model, a, "[B]");
a.changeElements(List.of(), List.of(b), "Test"); a.changeElements(List.of(), List.of(b), "Test");
assertEquals(b, waitOn(model.fetchModelObject("A", "[B]"))); assertEquals(b, waitOn(model.fetchModelValue("A", "[B]")));
} }
@Test @Test
@ -154,31 +155,36 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
FakeTargetObject b = new FakeTargetObject(model, a, "[B]"); FakeTargetObject b = new FakeTargetObject(model, a, "[B]");
a.changeElements(List.of(), List.of(b), "Test"); a.changeElements(List.of(), List.of(b), "Test");
assertEquals(null, waitOn(model.fetchModelObject("NoA", "[B]"))); assertEquals(null, waitOn(model.fetchModelValue("NoA", "[B]")));
assertEquals(null, waitOn(model.fetchModelObject("NoA", "[NoB]"))); assertEquals(null, waitOn(model.fetchModelValue("NoA", "[NoB]")));
assertEquals(null, waitOn(model.fetchModelObject("A", "[NoB]"))); assertEquals(null, waitOn(model.fetchModelValue("A", "[NoB]")));
} }
@Test @Test
public void testElementReplacement() throws Throwable { public void testElementReplacement() throws Throwable {
ElementsChangedListener elemL = new ElementsChangedListener(); ElementsChangedListener elemL = new ElementsChangedListener();
InvalidatedListener invL = new InvalidatedListener(); InvalidatedListener invL = new InvalidatedListener();
model.addModelListener(elemL);
model.addModelListener(invL);
TargetObjectAddedWaiter waiter = new TargetObjectAddedWaiter(model);
FakeTargetObject fakeA = new FakeTargetObject(model, model.root, "[A]"); FakeTargetObject fakeA = new FakeTargetObject(model, model.root, "[A]");
model.root.setElements(List.of(fakeA), "Init"); model.root.setElements(List.of(fakeA), "Init");
assertEquals(fakeA, waitOn(waiter.wait(PathUtils.parse("[A]"))));
model.root.addListener(elemL); waitOn(model.flushEvents());
fakeA.addListener(invL); elemL.clear();
invL.clear();
PhonyTargetObject phonyA = new PhonyTargetObject(model, model.root, "[A]"); PhonyTargetObject phonyA = new PhonyTargetObject(model, model.root, "[A]");
// mere creation causes removal of old // mere creation causes removal of old
waitOn(elemL.count.waitValue(1)); waitOn(elemL.count.waitUntil(c -> c >= 1));
ElementsChangedInvocation changed1 = Unique.assertOne(elemL.invocations); ElementsChangedInvocation changed1 = Unique.assertOne(elemL.invocations);
assertSame(model.root, changed1.parent); assertSame(model.root, changed1.parent);
assertEquals(Set.of("A"), changed1.removed); assertEquals(Set.of("A"), changed1.removed);
assertTrue(changed1.added.isEmpty()); assertTrue(changed1.added.isEmpty());
waitOn(invL.count.waitValue(1));
waitOn(invL.count.waitUntil(c -> c >= 1));
InvalidatedInvocation invalidated = Unique.assertOne(invL.invocations); InvalidatedInvocation invalidated = Unique.assertOne(invL.invocations);
assertSame(fakeA, invalidated.object); assertSame(fakeA, invalidated.object);
@ -186,38 +192,45 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
invL.clear(); invL.clear();
model.root.setElements(List.of(phonyA), "Replace"); model.root.setElements(List.of(phonyA), "Replace");
assertSame(phonyA, waitOn(model.fetchModelObject("[A]"))); assertSame(phonyA, waitOn(model.fetchModelValue("[A]")));
assertFalse(fakeA.isValid()); assertFalse(fakeA.isValid());
waitOn(elemL.count.waitUntil(c -> c >= 1));
ElementsChangedInvocation changed2 = Unique.assertOne(elemL.invocations); ElementsChangedInvocation changed2 = Unique.assertOne(elemL.invocations);
assertSame(model.root, changed2.parent); assertSame(model.root, changed2.parent);
assertSame(phonyA, Unique.assertOne(changed2.added.values())); assertSame(phonyA, Unique.assertOne(changed2.added.values()));
assertTrue(changed2.removed.isEmpty()); assertTrue(changed2.removed.isEmpty());
waiter.close();
} }
@Test @Test
public void testAttributeReplacement() throws Throwable { public void testAttributeReplacement() throws Throwable {
AttributesChangedListener attrL = new AttributesChangedListener(); AttributesChangedListener attrL = new AttributesChangedListener();
model.addModelListener(attrL);
String str1 = new String("EqualStrings"); String str1 = new String("EqualStrings");
String str2 = new String("EqualStrings"); String str2 = new String("EqualStrings");
model.root.setAttributes(Map.of("a", str1), "Init"); model.root.setAttributes(Map.of("a", str1), "Init");
model.root.addListener(attrL); waitOn(model.flushEvents());
// Note: mere object creation will cause "prior removal" /**
// We'll do this test just with primitives * Note: mere object creation will cause "prior removal," so we'll do this test just with
// Should not cause replacement, since they're equal * primitives. Should not cause replacement, since str1 and str2 are equal.
*/
attrL.clear();
model.root.setAttributes(Map.of("a", str2), "Replace"); model.root.setAttributes(Map.of("a", str2), "Replace");
waitOn(model.clientExecutor); waitOn(model.flushEvents());
// Verify str1 was not replaced with str2
assertSame(str1, waitOn(model.fetchModelValue("a"))); assertSame(str1, waitOn(model.fetchModelValue("a")));
assertEquals(0, attrL.invocations.size()); assertEquals(0, attrL.invocations.size());
// Now, with prior removal // Now, with prior removal
// TODO: Should I permit custom equality check? // TODO: Should I permit custom equality check?
attrL.clear();
model.root.setAttributes(Map.of(), "Clear"); model.root.setAttributes(Map.of(), "Clear");
model.root.setAttributes(Map.of("a", str2), "Replace"); model.root.setAttributes(Map.of("a", str2), "Replace");
waitOn(model.clientExecutor); waitOn(model.flushEvents());
assertEquals(2, attrL.invocations.size()); assertEquals(2, attrL.invocations.size());
AttributesChangedInvocation changed = attrL.invocations.get(0); AttributesChangedInvocation changed = attrL.invocations.get(0);
@ -233,6 +246,7 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
@Test @Test
public void testInvalidation() throws Throwable { public void testInvalidation() throws Throwable {
InvalidatedListener invL = new InvalidatedListener(); InvalidatedListener invL = new InvalidatedListener();
model.addModelListener(invL);
FakeTargetObject fakeA = new FakeTargetObject(model, model.root, "A"); FakeTargetObject fakeA = new FakeTargetObject(model, model.root, "A");
model.root.setAttributes(Map.of("A", fakeA), "Init"); model.root.setAttributes(Map.of("A", fakeA), "Init");
@ -240,10 +254,8 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
FakeTargetObject fakeA1 = new FakeTargetObject(model, fakeA, "[1]"); FakeTargetObject fakeA1 = new FakeTargetObject(model, fakeA, "[1]");
FakeTargetObject fakeA2 = new FakeTargetObject(model, fakeA, "[2]"); FakeTargetObject fakeA2 = new FakeTargetObject(model, fakeA, "[2]");
fakeA.setElements(List.of(fakeA1, fakeA2), "Init"); fakeA.setElements(List.of(fakeA1, fakeA2), "Init");
waitOn(model.flushEvents());
fakeA.addListener(invL); invL.clear();
fakeA1.addListener(invL);
fakeA2.addListener(invL);
model.root.setAttributes(Map.of(), "Clear"); model.root.setAttributes(Map.of(), "Clear");
@ -276,6 +288,11 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
} }
} }
@Override
public void rootAdded(TargetObject root) {
record.add(new ImmutablePair<>("rootAdded", root));
}
@Override @Override
public void registersUpdated(TargetObject bank, Map<String, byte[]> updates) { public void registersUpdated(TargetObject bank, Map<String, byte[]> updates) {
record.add(new ImmutablePair<>("registersUpdated", bank)); record.add(new ImmutablePair<>("registersUpdated", bank));
@ -287,6 +304,8 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
EventRecordingListener listener = new EventRecordingListener(); EventRecordingListener listener = new EventRecordingListener();
model.addModelListener(listener, false); model.addModelListener(listener, false);
waitOn(model.clientExecutor); waitOn(model.clientExecutor);
waitOn(model.fetchModelRoot());
listener.record.clear();
FakeTargetObject fakeA = new FakeTargetObject(model, model.root, "A"); FakeTargetObject fakeA = new FakeTargetObject(model, model.root, "A");
FakeTargetRegisterBank fakeA1rb = new FakeTargetRegisterBank(model, fakeA, "[1]"); FakeTargetRegisterBank fakeA1rb = new FakeTargetRegisterBank(model, fakeA, "[1]");
@ -296,10 +315,12 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
waitOn(model.clientExecutor); waitOn(model.clientExecutor);
assertEquals(List.of(new ImmutablePair<>("created", fakeA), assertEquals(List.of(
new ImmutablePair<>("created", fakeA),
new ImmutablePair<>("created", fakeA1rb), new ImmutablePair<>("created", fakeA1rb),
new ImmutablePair<>("registersUpdated", fakeA1rb), new ImmutablePair<>("registersUpdated", fakeA1rb),
new ImmutablePair<>("addedElem", fakeA1rb), new ImmutablePair<>("addedAttr", fakeA)), new ImmutablePair<>("addedElem", fakeA1rb),
new ImmutablePair<>("addedAttr", fakeA)),
listener.record); listener.record);
} }
@ -316,9 +337,13 @@ public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
waitOn(model.clientExecutor); waitOn(model.clientExecutor);
assertEquals(List.of(new ImmutablePair<>("created", model.root), assertEquals(List.of(
new ImmutablePair<>("created", fakeA), new ImmutablePair<>("created", fakeA1rb), new ImmutablePair<>("created", model.root),
new ImmutablePair<>("addedElem", fakeA1rb), new ImmutablePair<>("addedAttr", fakeA)), new ImmutablePair<>("created", fakeA),
new ImmutablePair<>("created", fakeA1rb),
new ImmutablePair<>("addedElem", fakeA1rb),
new ImmutablePair<>("addedAttr", fakeA),
new ImmutablePair<>("rootAdded", model.root)),
listener.record); listener.record);
} }
} }