diff --git a/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/target/Target.java b/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/target/Target.java index 4064db3b44..68473599f2 100644 --- a/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/target/Target.java +++ b/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. + * + *

+ * 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 does still need it may cause a host of other problems. + */ + void forciblyCloseTransactions(); } diff --git a/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/tracermi/TraceRmiConnection.java b/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/tracermi/TraceRmiConnection.java index 8e0ad284ff..9afc226bf0 100644 --- a/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/tracermi/TraceRmiConnection.java +++ b/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 + * + *

+ * 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); } diff --git a/Ghidra/Debug/Debugger-rmi-trace/src/main/help/help/topics/TraceRmiConnectionManagerPlugin/TraceRmiConnectionManagerPlugin.html b/Ghidra/Debug/Debugger-rmi-trace/src/main/help/help/topics/TraceRmiConnectionManagerPlugin/TraceRmiConnectionManagerPlugin.html index 9547a6a869..1462f39b71 100644 --- a/Ghidra/Debug/Debugger-rmi-trace/src/main/help/help/topics/TraceRmiConnectionManagerPlugin/TraceRmiConnectionManagerPlugin.html +++ b/Ghidra/Debug/Debugger-rmi-trace/src/main/help/help/topics/TraceRmiConnectionManagerPlugin/TraceRmiConnectionManagerPlugin.html @@ -105,5 +105,15 @@

Stop the server. This closes the persistent server. This does not affect pending acceptors or established connections.

+ +

Forcibly Close Transactions

+ +

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.

diff --git a/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/gui/tracermi/connection/TraceRmiConnectionManagerProvider.java b/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/gui/tracermi/connection/TraceRmiConnectionManagerProvider.java index f821e59bf8..515a7306f6 100644 --- a/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/gui/tracermi/connection/TraceRmiConnectionManagerProvider.java +++ b/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) { diff --git a/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/tracermi/TraceRmiHandler.java b/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/tracermi/TraceRmiHandler.java index 784d329075..1a7986fd9f 100644 --- a/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/tracermi/TraceRmiHandler.java +++ b/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,7 +1329,9 @@ public class TraceRmiHandler extends AbstractTraceRmiConnection { @Override public boolean isBusy() { - return !openTxes.isEmpty(); + synchronized (openTxes) { + return !openTxes.isEmpty(); + } } @Override @@ -1335,11 +1341,33 @@ public class TraceRmiHandler extends AbstractTraceRmiConnection { return false; } - for (Tid tid : openTxes.keySet()) { - if (Objects.equals(openTrace.doId, tid.doId)) { - return true; + 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); + } } diff --git a/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/tracermi/TraceRmiTarget.java b/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/tracermi/TraceRmiTarget.java index 779643b1a5..b12a86067c 100644 --- a/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/service/tracermi/TraceRmiTarget.java +++ b/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); + } } diff --git a/Ghidra/Debug/Debugger-rmi-trace/src/test/java/ghidra/app/plugin/core/debug/service/tracermi/TestTraceRmiConnection.java b/Ghidra/Debug/Debugger-rmi-trace/src/test/java/ghidra/app/plugin/core/debug/service/tracermi/TestTraceRmiConnection.java index 4a35aaae56..a67663468d 100644 --- a/Ghidra/Debug/Debugger-rmi-trace/src/test/java/ghidra/app/plugin/core/debug/service/tracermi/TestTraceRmiConnection.java +++ b/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) { + } } diff --git a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/MockTarget.java b/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/MockTarget.java index 4f0ebe5bd8..fbc4fc7b0a 100644 --- a/Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/service/MockTarget.java +++ b/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() { + } }