GP-0: Fixing test fallout from GP-690

Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/DebuggerModelFactory.java

@@ -21,6 +21,7 @@ import ghidra.util.classfinder.ExtensionPoint;
 /**
  * A factory for a debugger model
  * 
+ * <p>
  * This provides a discoverable means of creating a debug model.
  */
 public interface DebuggerModelFactory

Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/DebuggerObjectModel.java

@@ -509,4 +509,39 @@ public interface DebuggerObjectModel {
 	 * @return the executor
 	 */
 	Executor getClientExecutor();
+
+	/**
+	 * Ensure that dependent computations occur on the client executor
+	 * 
+	 * <p>
+	 * This also preserves scheduling order on the executor. Using just
+	 * {@link CompletableFuture#thenApplyAsync(java.util.function.Function)} makes no guarantees
+	 * about execution order, because that invocation could occur before invocations in the chained
+	 * actions. This one instead uses
+	 * {@link CompletableFuture#thenCompose(java.util.function.Function)} to schedule a final action
+	 * which performs the actual completion via the executor.
+	 * 
+	 * @param <T> the type of the future value
+	 * @param cf the future
+	 * @return a future gated via the client executor
+	 */
+	default <T> CompletableFuture<T> gateFuture(CompletableFuture<T> cf) {
+		return cf.thenCompose(this::gateFuture);
+	}
+
+	/**
+	 * Ensure that dependent computations occur on the client executor
+	 * 
+	 * <p>
+	 * Use as a method reference in a final call to
+	 * {@link CompletableFuture#thenCompose(java.util.function.Function)} to ensure the final stage
+	 * completes on the client executor.
+	 * 
+	 * @param <T> the type of the future value
+	 * @param v the value
+	 * @return a future while completes with the given value on the client executor
+	 */
+	default <T> CompletableFuture<T> gateFuture(T v) {
+		return CompletableFuture.supplyAsync(() -> v, getClientExecutor());
+	}
 }

Ghidra/Debug/Framework-Debugging/src/main/java/ghidra/dbg/agent/DefaultTargetObject.java

@@ -163,7 +163,7 @@ public class DefaultTargetObject<E extends TargetObject, P extends TargetObject>
 			}
 			req = curElemsRequest;
 		}
-		return req.thenApply(__ -> getCachedElements());
+		return req.thenApply(__ -> getCachedElements()).thenCompose(model::gateFuture);
 	}
 
 	@Override
@@ -337,7 +337,7 @@ public class DefaultTargetObject<E extends TargetObject, P extends TargetObject>
 				}
 				return getCachedAttributes();
 			}
-		});
+		}).thenCompose(model::gateFuture);
 	}
 
 	@Override

Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/agent/DefaultDebuggerObjectModelTest.java

@@ -20,11 +20,12 @@ import static org.junit.Assert.*;
 
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.*;
+import java.util.concurrent.CompletableFuture;
 
 import org.junit.Test;
 
 import generic.Unique;
+import ghidra.async.AsyncTestUtils;
 import ghidra.async.AsyncUtils;
 import ghidra.dbg.DebuggerObjectModel;
 import ghidra.dbg.target.TargetObject;
@@ -34,11 +35,8 @@ import ghidra.dbg.util.ElementsChangedListener.ElementsChangedInvocation;
 import ghidra.dbg.util.InvalidatedListener.InvalidatedInvocation;
 import ghidra.program.model.address.AddressFactory;
 import ghidra.program.model.address.AddressSpace;
-import ghidra.util.SystemUtilities;
 
-public class DefaultDebuggerObjectModelTest {
-	static final long TIMEOUT_MILLISECONDS =
-		SystemUtilities.isInTestingBatchMode() ? 5000 : Long.MAX_VALUE;
+public class DefaultDebuggerObjectModelTest implements AsyncTestUtils {
 
 	public static class FakeTargetObject extends DefaultTargetObject<TargetObject, TargetObject> {
 		public FakeTargetObject(DebuggerObjectModel model, TargetObject parent, String name) {
@@ -92,15 +90,6 @@ public class DefaultDebuggerObjectModelTest {
 		}
 	}
 
-	protected static <T> T waitOn(CompletableFuture<T> future) throws Throwable {
-		try {
-			return future.get(TIMEOUT_MILLISECONDS, TimeUnit.MILLISECONDS);
-		}
-		catch (ExecutionException e) {
-			throw e.getCause();
-		}
-	}
-
 	FakeDebuggerObjectModel model = new FakeDebuggerObjectModel();
 
 	@Test
@@ -187,6 +176,7 @@ public class DefaultDebuggerObjectModelTest {
 
 		PhonyTargetObject phonyA = new PhonyTargetObject(model, model.root, "A");
 		model.root.setAttributes(Map.of("A", phonyA), "Replace");
+		waitOn(model.clientExecutor);
 
 		// Object-valued attribute replacement requires prior removal 
 		assertSame(fakeA, waitOn(model.fetchModelObject("A")));
@@ -197,6 +187,7 @@ public class DefaultDebuggerObjectModelTest {
 		// TODO: Should I permit custom equality check?
 		model.root.setAttributes(Map.of(), "Clear");
 		model.root.setAttributes(Map.of("A", phonyA), "Replace");
+		waitOn(model.clientExecutor);
 
 		assertEquals(2, attrL.invocations.size());
 		AttributesChangedInvocation changed = attrL.invocations.get(0);

Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/AbstractTestTargetRegisterBank.java

@@ -57,7 +57,7 @@ public abstract class AbstractTestTargetRegisterBank<T extends AbstractTestTarge
 		return regs.getModel().future(result).thenApply(__ -> {
 			listeners.fire(TargetRegisterBankListener.class).registersUpdated(this, result);
 			return result;
-		});
+		}).thenCompose(model::gateFuture);
 	}
 
 	protected CompletableFuture<Void> writeRegs(Map<String, byte[]> values,
@@ -81,7 +81,7 @@ public abstract class AbstractTestTargetRegisterBank<T extends AbstractTestTarge
 		}
 		future.thenAccept(__ -> {
 			listeners.fire(TargetRegisterBankListener.class).registersUpdated(this, updates);
-		});
+		}).thenCompose(model::gateFuture);
 		return future;
 	}
 

Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestDebuggerObjectModel.java

@@ -32,7 +32,7 @@ public class TestDebuggerObjectModel extends AbstractDebuggerObjectModel {
 		CompletableFuture.delayedExecutor(DELAY_MILLIS, TimeUnit.MILLISECONDS);
 
 	enum FutureMode {
-		SYNC, ASYNC, DELAYED;
+		ASYNC, DELAYED;
 	}
 
 	protected final AddressSpace ram =
@@ -40,8 +40,6 @@ public class TestDebuggerObjectModel extends AbstractDebuggerObjectModel {
 	protected final AddressFactory factory = new DefaultAddressFactory(new AddressSpace[] { ram });
 	public final TestTargetSession session;
 
-	protected TestDebuggerObjectModel.FutureMode futureMode = FutureMode.SYNC;
-
 	protected int invalidateCachesCount;
 
 	public TestDebuggerObjectModel() {
@@ -78,15 +76,7 @@ public class TestDebuggerObjectModel extends AbstractDebuggerObjectModel {
 	}
 
 	public <T> CompletableFuture<T> future(T t) {
-		switch (futureMode) {
-			case SYNC:
-				return CompletableFuture.completedFuture(t);
-			case ASYNC:
-				return CompletableFuture.supplyAsync(() -> t);
-			case DELAYED:
-				return CompletableFuture.supplyAsync(() -> t, DELAYED_EXECUTOR);
-		}
-		throw new AssertionError();
+		return CompletableFuture.supplyAsync(() -> t, getClientExecutor());
 	}
 
 	public CompletableFuture<Void> requestFocus(TargetObjectRef obj) {

Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/model/TestTargetSession.java

@@ -64,7 +64,7 @@ public class TestTargetSession extends DefaultTargetModelRoot
 				FOCUS_ATTRIBUTE_NAME, obj //
 			), "Focus requested");
 			listeners.fire(TargetFocusScopeListener.class).focusChanged(this, obj);
-		});
+		}).thenCompose(model::gateFuture);
 	}
 
 	public void simulateStep(TestTargetThread eventThread) {