GP-5788: Add an action to forcibly close all transactions for a target (in case of back-end misbehavior)

Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/target/Target.java

@@ -700,4 +700,15 @@ public interface Target {
 	 * @return true if busy
 	 */
 	boolean isBusy();
+
+	/**
+	 * Forcibly commit all of the back-ends transactions on this target's trace.
+	 * 
+	 * <p>
+	 * This is generally not a recommended course of action, except that sometimes the back-end
+	 * crashes and fails to close a transaction. It should only be invoked by a relatively hidden
+	 * menu option, and mediated by a warning of some sort. Closing a transaction prematurely, when
+	 * the back-end actually <em>does</em> still need it may cause a host of other problems.
+	 */
+	void forciblyCloseTransactions();
 }

Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/tracermi/TraceRmiConnection.java

@@ -171,8 +171,19 @@ public interface TraceRmiConnection extends AutoCloseable {
 	/**
 	 * Check if the given target has a transaction open
 	 * 
-	 * @param target
+	 * @param target the target
 	 * @return true if busy
 	 */
 	boolean isBusy(Target target);
+
+	/**
+	 * Forcibly commit all transactions this connection has on the given trace
+	 * 
+	 * <p>
+	 * This may cause undefined behavior in the back-end, especially if it still needs the
+	 * transaction.
+	 * 
+	 * @param target the the target
+	 */
+	void forciblyCloseTransactions(Target target);
 }

Ghidra/Debug/Debugger-rmi-trace/src/main/help/help/topics/TraceRmiConnectionManagerPlugin/TraceRmiConnectionManagerPlugin.html

@@ -105,5 +105,15 @@
 
     <P>Stop the server. This closes the persistent server. This does not affect pending acceptors
     or established connections.</P>
+
+    <H3><A name="forcibly_close_txes"></A>Forcibly Close Transactions</H3>
+
+    <P>Forcibly close all the back-end's transactions on the target trace. This is generally not a
+    recommended course of action, except that sometimes the back-end crashes and fails to close a
+    transaction. Un-closed transactions from the back-end can leave most, if not all, of the UI in
+    a stale state, since event processing on the trace is disabled. If there is good reason to
+    believe the back-end has forgotten to close a transaction, this action will forcibly close all
+    of them and re-enable event processing. If, however, the back-end was in fact still doing work
+    with that transaction, it may crash and/or corrupt the connection.</P>
   </BODY>
 </HTML>

Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/gui/tracermi/connection/TraceRmiConnectionManagerProvider.java

@@ -161,6 +161,24 @@ public class TraceRmiConnectionManagerProvider extends ComponentProviderAdapter
 		}
 	}
 
+	interface ForceCloseTransactionsActions {
+		String NAME = "Forcibly Close Transactions";
+		String DESCRIPTION = "Forcibly commit all remote transactions on the trace";
+		String GROUP = GROUP_MAINTENANCE;
+		String HELP_ANCHOR = "forcibly_close_txes";
+
+		static ActionBuilder builder(Plugin owner) {
+			String ownerName = owner.getName();
+			return new ActionBuilder(NAME, ownerName)
+					.description(DESCRIPTION)
+					.menuPath(NAME)
+					.popupMenuPath(NAME)
+					.menuGroup(GROUP)
+					.popupMenuGroup(GROUP)
+					.helpLocation(new HelpLocation(ownerName, HELP_ANCHOR));
+		}
+	}
+
 	class InjectableGTree extends GTree {
 		public InjectableGTree(GTreeNode root) {
 			super(root);
@@ -200,6 +218,7 @@ public class TraceRmiConnectionManagerProvider extends ComponentProviderAdapter
 	DockingAction actionConnectOutbound;
 	DockingAction actionCloseConnection;
 	DockingAction actionCloseAll;
+	DockingAction actionForceCloseTransactions;
 
 	TraceRmiManagerActionContext myActionContext;
 
@@ -309,6 +328,12 @@ public class TraceRmiConnectionManagerProvider extends ComponentProviderAdapter
 				.enabledWhen(this::isActionCloseAllEnabled)
 				.onAction(this::doActionCloseAllActivated)
 				.buildAndInstallLocal(this);
+
+		actionForceCloseTransactions = ForceCloseTransactionsActions.builder(plugin)
+				.withContext(TraceRmiManagerActionContext.class)
+				.enabledWhen(this::isActionForceCloseTransactionsEnabled)
+				.onAction(this::doActionCloseTransactionsActivated)
+				.buildAndInstallLocal(this);
 	}
 
 	@Override
@@ -482,6 +507,21 @@ public class TraceRmiConnectionManagerProvider extends ComponentProviderAdapter
 		}
 	}
 
+	private boolean isActionForceCloseTransactionsEnabled(TraceRmiManagerActionContext context) {
+		TraceRmiManagerNode node = context.getSelectedNode();
+		if (node instanceof TraceRmiTargetNode) {
+			return true;
+		}
+		return false;
+	}
+
+	private void doActionCloseTransactionsActivated(TraceRmiManagerActionContext context) {
+		TraceRmiManagerNode node = context.getSelectedNode();
+		if (node instanceof TraceRmiTargetNode tNode) {
+			tNode.getTarget().forciblyCloseTransactions();
+		}
+	}
+
 	@AutoServiceConsumed
 	private void setTraceRmiService(TraceRmiService traceRmiService) {
 		if (this.traceRmiService != null) {

Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/tracermi/TraceRmiHandler.java

@@ -946,6 +946,18 @@ public class TraceRmiHandler extends AbstractTraceRmiConnection {
 				.build();
 	}
 
+	protected void checkRestoreEvents(OpenTrace open) {
+		final boolean restoreEvents;
+		synchronized (openTxes) {
+			restoreEvents = openTxes.keySet()
+					.stream()
+					.noneMatch(id -> id.doId.equals(open.doId));
+		}
+		if (restoreEvents) {
+			open.trace.setEventsEnabled(true);
+		}
+	}
+
 	protected ReplyEndTx handleEndTx(RequestEndTx req) {
 		OpenTx tx;
 		synchronized (openTxes) {
@@ -969,16 +981,8 @@ public class TraceRmiHandler extends AbstractTraceRmiConnection {
 		}
 
 		tx.tx.close();
+		checkRestoreEvents(open);
 
-		final boolean restoreEvents;
-		synchronized (openTxes) {
-			restoreEvents = openTxes.keySet()
-					.stream()
-					.noneMatch(id -> id.doId.domObjId == req.getOid().getId());
-		}
-		if (restoreEvents) {
-			open.trace.setEventsEnabled(true);
-		}
 		Swing.runLater(
 			() -> plugin.listeners.invoke().transactionClosed(this, open.target, req.getAbort()));
 		return ReplyEndTx.getDefaultInstance();
@@ -1325,8 +1329,10 @@ public class TraceRmiHandler extends AbstractTraceRmiConnection {
 
 	@Override
 	public boolean isBusy() {
+		synchronized (openTxes) {
 			return !openTxes.isEmpty();
 		}
+	}
 
 	@Override
 	public boolean isBusy(Target target) {
@@ -1335,11 +1341,33 @@ public class TraceRmiHandler extends AbstractTraceRmiConnection {
 			return false;
 		}
 
+		synchronized (openTxes) {
 			for (Tid tid : openTxes.keySet()) {
 				if (Objects.equals(openTrace.doId, tid.doId)) {
 					return true;
 				}
 			}
+		}
 		return false;
 	}
+
+	@Override
+	public void forciblyCloseTransactions(Target target) {
+		OpenTrace open = openTraces.getByTrace(target.getTrace());
+		if (open == null || open.target != target) {
+			return;
+		}
+		synchronized (openTxes) {
+			for (OpenTx tx : List.copyOf(openTxes.values())) {
+				if (Objects.equals(open.doId, tx.txId.doId)) {
+					openTxes.remove(tx.txId);
+					tx.tx.commit();
+					tx.tx.close();
+					Swing.runLater(
+						() -> plugin.listeners.invoke().transactionClosed(this, target, false));
+				}
+			}
+		}
+		open.trace.setEventsEnabled(true);
+	}
 }

Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/tracermi/TraceRmiTarget.java

@@ -1736,4 +1736,9 @@ public class TraceRmiTarget extends AbstractTarget {
 	public boolean isBusy() {
 		return connection.isBusy(this);
 	}
+
+	@Override
+	public void forciblyCloseTransactions() {
+		connection.forciblyCloseTransactions(this);
+	}
 }

Ghidra/Debug/Debugger-rmi-trace/src/test/java/ghidra/app/plugin/core/debug/service/tracermi/TestTraceRmiConnection.java

@@ -277,4 +277,8 @@ public abstract class TestTraceRmiConnection extends AbstractTraceRmiConnection
 	public boolean isBusy(Target target) {
 		return false;
 	}
+
+	@Override
+	public void forciblyCloseTransactions(Target target) {
+	}
 }

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/MockTarget.java

@@ -278,4 +278,8 @@ public class MockTarget implements Target {
 	public boolean isBusy() {
 		return false;
 	}
+
+	@Override
+	public void forciblyCloseTransactions() {
+	}
 }