From 5b7c1e3119050071a97bda35636f7ee4974af9af Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Fri, 6 Sep 2024 14:59:43 -0400 Subject: [PATCH] GP-0: Fix tests. --- .../service/tracermi/TraceRmiHandler.java | 41 ++++++++++++++----- .../gui/AbstractDebuggerParameterDialog.java | 5 +++ .../model/DebuggerModelServicePlugin.java | 20 ++++++--- 3 files changed, 50 insertions(+), 16 deletions(-) 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 721862cefb..d05eab6408 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 @@ -188,6 +188,18 @@ public class TraceRmiHandler implements TraceRmiConnection { .collect(Collectors.toUnmodifiableList()); } + /** + * Call only for cleanup. Cannot be re-used after this + * + * @return the open traces that were removed + */ + public synchronized List clearAll() { + List all = List.copyOf(byId.values()); + byId.clear(); + byTrace.clear(); + return all; + } + public CompletableFuture getFirstAsync() { return first; } @@ -273,14 +285,15 @@ public class TraceRmiHandler implements TraceRmiConnection { terminateTerminals(); socket.close(); - while (!openTxes.isEmpty()) { - Tid nextKey = openTxes.keySet().iterator().next(); - OpenTx open = openTxes.remove(nextKey); - open.tx.close(); + synchronized (openTxes) { + while (!openTxes.isEmpty()) { + Tid nextKey = openTxes.keySet().iterator().next(); + OpenTx open = openTxes.remove(nextKey); + open.tx.close(); + } } - while (!openTraces.isEmpty()) { - DoId nextKey = openTraces.idSet().iterator().next(); - OpenTrace open = openTraces.removeById(nextKey); + + for (OpenTrace open : openTraces.clearAll()) { if (traceManager == null || traceManager.isSaveTracesByDefault()) { try (CloseableTaskMonitor monitor = plugin.createMonitor()) { open.trace.save("Save on Disconnect", monitor); @@ -610,7 +623,10 @@ public class TraceRmiHandler implements TraceRmiConnection { } protected Tid requireAvailableTid(Tid tid) { - OpenTx tx = openTxes.get(tid); + OpenTx tx; + synchronized (openTxes) { + tx = openTxes.get(tid); + } if (tx != null) { throw new TxIdInUseError(); } @@ -946,7 +962,10 @@ public class TraceRmiHandler implements TraceRmiConnection { } protected ReplyEndTx handleEndTx(RequestEndTx req) { - OpenTx tx = openTxes.remove(new Tid(new DoId(req.getOid()), req.getTxid().getId())); + OpenTx tx; + synchronized (openTxes) { + tx = openTxes.remove(new Tid(new DoId(req.getOid()), req.getTxid().getId())); + } if (tx == null) { throw new InvalidTxIdError(req.getTxid().getId()); } @@ -1159,7 +1178,9 @@ public class TraceRmiHandler implements TraceRmiConnection { @SuppressWarnings("resource") OpenTx tx = new OpenTx(tid, open.trace.openTransaction(req.getDescription()), req.getUndoable()); - openTxes.put(tx.txId, tx); + synchronized (openTxes) { + openTxes.put(tx.txId, tx); + } return ReplyStartTx.getDefaultInstance(); } diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/AbstractDebuggerParameterDialog.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/AbstractDebuggerParameterDialog.java index 8b229e15af..d9ab6d9435 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/AbstractDebuggerParameterDialog.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/AbstractDebuggerParameterDialog.java @@ -700,6 +700,11 @@ public abstract class AbstractDebuggerParameterDialog

extends DialogComponent return; } memorized.put(parameterNameAndType(parameter), value); + PropertyEditor editor = paramEditors.get(parameter); + // Editors may not be populated yet + if (editor != null) { + setEditorValue(editor, parameter, value); + } } public void forgetMemorizedArguments() { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DebuggerModelServicePlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DebuggerModelServicePlugin.java index eea5e338a4..01a561f8a7 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DebuggerModelServicePlugin.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/model/DebuggerModelServicePlugin.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -37,7 +37,8 @@ import ghidra.app.plugin.PluginCategoryNames; import ghidra.app.plugin.core.debug.DebuggerPluginPackage; import ghidra.app.plugin.core.debug.gui.DebuggerResources.DisconnectAllAction; import ghidra.app.plugin.core.debug.service.model.launch.DebuggerProgramLaunchOpinion; -import ghidra.app.services.*; +import ghidra.app.services.DebuggerModelService; +import ghidra.app.services.DebuggerTraceManagerService; import ghidra.app.services.DebuggerTraceManagerService.ActivationCause; import ghidra.async.AsyncFence; import ghidra.dbg.*; @@ -380,10 +381,17 @@ public class DebuggerModelServicePlugin extends Plugin protected void removeRecorder(TraceRecorder recorder) { synchronized (recordersByTarget) { TraceRecorder old = recordersByTarget.remove(recorder.getTarget()); - if (old != recorder) { - throw new AssertionError("Container-recorder mix up"); + /** + * Possible race condition when quickly launching and stopping a recording. If it's + * already removed, that's actually fine. If we get something non-null that doesn't + * match, then yeah, something's truly gone wrong. + */ + if (old != null) { + if (old != recorder) { + throw new AssertionError("Container-recorder mix up"); + } + old.removeListener(listenerOnRecorders); } - old.removeListener(listenerOnRecorders); } recorderListeners.invoke().elementRemoved(recorder); }