diff --git a/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/UnixShellScriptTraceRmiLaunchOffer.java b/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/UnixShellScriptTraceRmiLaunchOffer.java index bb9a379cd0..052b3d9dfa 100644 --- a/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/UnixShellScriptTraceRmiLaunchOffer.java +++ b/Ghidra/Debug/Debugger-rmi-trace/src/main/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/UnixShellScriptTraceRmiLaunchOffer.java @@ -37,10 +37,13 @@ public class UnixShellScriptTraceRmiLaunchOffer extends AbstractScriptTraceRmiLa /** * Create a launch offer from the given shell script. * + * @param plugin the launcher service plugin * @param program the current program, usually the target image. In general, this should be used * for at least two purposes. 1) To populate the default command line. 2) To ensure * the target image is mapped in the resulting target trace. - * @throws FileNotFoundException + * @param script the script file that implements this offer + * @return the offer + * @throws FileNotFoundException if the script file does not exist */ public static UnixShellScriptTraceRmiLaunchOffer create(TraceRmiLauncherServicePlugin plugin, Program program, File script) throws FileNotFoundException { 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 52c9f9ae63..a3c3b92877 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 @@ -225,6 +225,7 @@ public class TraceRmiHandler implements TraceRmiConnection { *

* Note it is common for this to be constructed by a TCP client. * + * @param plugin the Trace RMI plugin * @param socket the socket to the back-end debugger * @throws IOException if there is an issue with the I/O streams */ diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointMarkerPlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointMarkerPlugin.java index fbcafb5079..6c654c0789 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointMarkerPlugin.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointMarkerPlugin.java @@ -513,7 +513,7 @@ public class DebuggerBreakpointMarkerPlugin extends Plugin return null; } } - + protected static State computeState(LogicalBreakpoint breakpoint, Program programOrView) { if (programOrView instanceof TraceProgramView view) { return breakpoint.computeStateForTrace(view.getTrace()); @@ -809,7 +809,7 @@ public class DebuggerBreakpointMarkerPlugin extends Plugin BreakpointsDecompilerMarginProvider decompilerMarginProvider; private MarginProviderSupplier functionGraphMarginSupplier = - new DefaultMarginProviderSupplier(); + new DefaultMarginProviderSupplier(); public DebuggerBreakpointMarkerPlugin(PluginTool tool) { super(tool); @@ -973,7 +973,9 @@ public class DebuggerBreakpointMarkerPlugin extends Plugin protected void removeMarkers(Program program) { synchronized (markersByProgram) { BreakpointMarkerSets oldSets = markersByProgram.remove(program); - oldSets.dispose(); + if (oldSets != null) { + oldSets.dispose(); + } } } @@ -1069,7 +1071,7 @@ public class DebuggerBreakpointMarkerPlugin extends Plugin this.functionGraphMarginService.addMarkerProviderSupplier(functionGraphMarginSupplier); } } - + protected void createActions() { actionSetSoftwareBreakpoint = new SetBreakpointAction(Set.of(TraceBreakpointKind.SW_EXECUTE)); diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java index 5a71334840..d5b14e6a51 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java @@ -434,8 +434,8 @@ public class DebuggerListingProvider extends CodeViewerProvider { * is synchronized exactly with the other providers. "Main" on the other hand, does not * necessarily have that property, but it is still not a snapshot. It is the main * listing presented by this plugin, and so it has certain unique features. Calling - * {@link DebuggerListingPlugin#getConnectedProvider()} will return the main dynamic listing, - * despite it not really being "connected." + * {@link DebuggerListingPlugin#getProvider()} will return the main dynamic listing, despite it + * not really being "connected." * * @return true if this is the main listing for the plugin. */ @@ -649,8 +649,12 @@ public class DebuggerListingProvider extends CodeViewerProvider { @Override protected void doSetProgram(Program newProgram) { + // E.g., The "Navigate Previous" could cause a change in trace if (newProgram != null && current.getView() != null && newProgram != current.getView()) { - throw new AssertionError(); + if (!(newProgram instanceof TraceProgramView view)) { + throw new IllegalArgumentException("Dynamic Listings require trace views"); + } + traceManager.activateTrace(view.getTrace()); } if (getProgram() == newProgram) { return; diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/memory/DebuggerMemoryBytesProvider.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/memory/DebuggerMemoryBytesProvider.java index d1ecc34dfb..a9c12df03b 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/memory/DebuggerMemoryBytesProvider.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/memory/DebuggerMemoryBytesProvider.java @@ -426,6 +426,9 @@ public class DebuggerMemoryBytesProvider extends ProgramByteViewerComponentProvi current = coordinates; addNewListeners(); doSetProgram(current.getView()); + // NB. Also avoid a stale location being reported to the history service. + setLocation(null); + setSelection(null, false); goToTrait.goToCoordinates(coordinates); trackingTrait.goToCoordinates(coordinates); readsMemTrait.goToCoordinates(coordinates); diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java index d936c3cf24..ed97bd8ef5 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/tracemgr/DebuggerTraceManagerServicePlugin.java @@ -75,7 +75,10 @@ import ghidra.util.task.*; packageName = DebuggerPluginPackage.NAME, status = PluginStatus.RELEASED, eventsProduced = { + TraceOpenedPluginEvent.class, TraceActivatedPluginEvent.class, + TraceInactiveCoordinatesPluginEvent.class, + TraceClosedPluginEvent.class, }, eventsConsumed = { TraceActivatedPluginEvent.class, @@ -284,6 +287,8 @@ public class DebuggerTraceManagerServicePlugin extends Plugin private DebuggerPlatformService platformService; // @AutoServiceConsumed via method private DebuggerControlService controlService; + @AutoServiceConsumed + private NavigationHistoryService navigationHistoryService; @SuppressWarnings("unused") private final AutoService.Wiring autoServiceWiring; @@ -959,6 +964,9 @@ public class DebuggerTraceManagerServicePlugin extends Plugin } protected void doTraceClosed(Trace trace) { + if (navigationHistoryService != null) { + navigationHistoryService.clear(trace.getProgramView()); + } synchronized (listenersByTrace) { trace.release(this); lastCoordsByTrace.remove(trace); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NavigationHistoryPlugin.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NavigationHistoryPlugin.java index e35c40c081..8d38294c49 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NavigationHistoryPlugin.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/NavigationHistoryPlugin.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. @@ -348,7 +348,8 @@ public class NavigationHistoryPlugin extends Plugin notifyHistoryChange(); } - private void clear(Program program) { + @Override + public void clear(Program program) { for (HistoryList historyList : historyListMap.values()) { clear(historyList, program); } @@ -657,7 +658,7 @@ public class NavigationHistoryPlugin extends Plugin ProgramLocation location = validateProgramLocation(activeNavigatable, memento.getProgramLocation()); if (location != null) { - // prefer the active natigatable's location + // prefer the active navigatable's location return location; } } @@ -683,11 +684,13 @@ public class NavigationHistoryPlugin extends Plugin Program program = location.getProgram(); if (program.isClosed()) { - Msg.showError(this, null, "Closed Program", - "The Navigation History Plugin is using a closed program.\n" + "Program: " + - program.getName() + "Navigatable: " + - navigatable.getClass().getSimpleName() + "\n" + "Location: " + - location.getClass().getSimpleName() + " @ " + location.getAddress()); + Msg.showError(this, null, "Closed Program", """ + The Navigation History Plugin is using a closed program. + Program: %s + Navigatable: %s + Location: %s @ %s""" + .formatted(program.getName(), navigatable.getClass().getSimpleName(), + location.getClass().getSimpleName(), location.getAddress())); return true; } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/services/NavigationHistoryService.java b/Ghidra/Features/Base/src/main/java/ghidra/app/services/NavigationHistoryService.java index 969c93af07..d25c072f6c 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/services/NavigationHistoryService.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/services/NavigationHistoryService.java @@ -21,65 +21,71 @@ import ghidra.app.nav.LocationMemento; import ghidra.app.nav.Navigatable; import ghidra.app.plugin.core.navigation.NavigationHistoryPlugin; import ghidra.framework.plugintool.ServiceInfo; +import ghidra.program.model.listing.Program; /** - * The ToolStateHistoryService maintains a stack of locations that the user - * has visited via a navigation plugin. - * It provides methods querying and manipulating this list. + * The NavigationHistoryService maintains a stack of locations that the user has visited via a + * navigation plugin. It provides methods querying and manipulating this list. */ -@ServiceInfo(defaultProvider = NavigationHistoryPlugin.class, description = "Maintains a history of tool states") +@ServiceInfo( + defaultProvider = NavigationHistoryPlugin.class, + description = "Maintains a history of tool states") public interface NavigationHistoryService { /** - * Positions the current location to the next location in the history list. - * If there is no "next" location, the history list remains unchanged. + * Positions the current location to the next location in the history list. If there is no + * "next" location, the history list remains unchanged. + * * @param navigatable the navigatable to be navigated */ public void next(Navigatable navigatable); /** - * Positions the "current" location to the previous location in the history list. - * If there is no "previous" location, the history list remains unchanged. + * Positions the "current" location to the previous location in the history list. If there is no + * "previous" location, the history list remains unchanged. + * * @param navigatable the navigatable to be navigated */ public void previous(Navigatable navigatable); - /** - * Navigates to the given location in the "next" list. If the location is not in the list, then + /** + * Navigates to the given location in the "next" list. If the location is not in the list, then * nothing will happen. * * @param navigatable the navigatable to be navigated - * @param location The location within the "next" list to which to go + * @param location The location within the "next" list to which to go */ public void next(Navigatable navigatable, LocationMemento location); - /** - * Navigates to the given location in the "previous" list. If the location is not in - * the list, then nothing will happen + /** + * Navigates to the given location in the "previous" list. If the location is not in the list, + * then nothing will happen * * @param navigatable the navigatable to be navigated - * @param location The location within the "previous" list to which to go. + * @param location The location within the "previous" list to which to go. */ public void previous(Navigatable navigatable, LocationMemento location); /** - * Positions the "current" location to the next location which is in a different function - * from current one or previous non-code location. - * If we are not inside any function, performs like "next". + * Positions the "current" location to the next location which is in a different function from + * current one or previous non-code location. If we are not inside any function, performs like + * "next". + * * @param navigatable the navigatable to be navigated */ public void nextFunction(Navigatable navigatable); /** - * Positions the "previous" location to the next location which is in a different function - * from current one or previous non-code location. - * If we are not inside any function, performs like "next". + * Positions the "previous" location to the next location which is in a different function from + * current one or previous non-code location. If we are not inside any function, performs like + * "next". + * * @param navigatable the navigatable to be navigated */ public void previousFunction(Navigatable navigatable); /** - * Returns the LocationMemento objects in the "previous" list + * Returns the {@link LocationMemento} objects in the "previous" list * * @param navigatable the navigatable to be navigated * @return the LocationMemento objects in the "previous" list @@ -87,7 +93,7 @@ public interface NavigationHistoryService { public List getPreviousLocations(Navigatable navigatable); /** - * Returns the LocationMemento objects in the "next" list + * Returns the {@link LocationMemento} objects in the "next" list * * @param navigatable the navigatable to be navigated * @return the LocationMemento objects in the "next" list @@ -112,30 +118,39 @@ public interface NavigationHistoryService { /** * Returns true if there is a valid "next" function location in the history list + * * @param navigatable Navigatable object we are looking at - * @return true if there is a valid "next" function location + * @return true if there is a valid "next" function location */ public boolean hasNextFunction(Navigatable navigatable); /** * Returns true if there is a valid "previous" function location in the history list + * * @param navigatable Navigatable object we are looking at - * @return true if there is a valid "previous" function location + * @return true if there is a valid "previous" function location */ public boolean hasPreviousFunction(Navigatable navigatable); /** - * Adds the given locationMomento to the list of previous locations. Clears the list - * of next locations. + * Adds the current location memento to the list of previous locations for the given + * navigatable. Clears the list of next locations. * * @param navigatable the navigatable to be navigated */ public void addNewLocation(Navigatable navigatable); /** - * Removes all visited locations from the history list + * Removes all visited locations from the history list for the given navigatable * - * @param navigatable the navigatable to be navigated + * @param navigatable the navigatable whose list to be cleared */ public void clear(Navigatable navigatable); + + /** + * Removes all entries for the given program from all history lists + * + * @param program the program whose entries to be cleared + */ + public void clear(Program program); } diff --git a/Ghidra/Features/ByteViewer/src/main/java/ghidra/app/plugin/core/byteviewer/ProgramByteViewerComponentProvider.java b/Ghidra/Features/ByteViewer/src/main/java/ghidra/app/plugin/core/byteviewer/ProgramByteViewerComponentProvider.java index 1f32637478..ea655907c5 100644 --- a/Ghidra/Features/ByteViewer/src/main/java/ghidra/app/plugin/core/byteviewer/ProgramByteViewerComponentProvider.java +++ b/Ghidra/Features/ByteViewer/src/main/java/ghidra/app/plugin/core/byteviewer/ProgramByteViewerComponentProvider.java @@ -170,7 +170,7 @@ public class ProgramByteViewerComponentProvider extends ByteViewerComponentProvi return getCurrentTextSelection(); } - private void setSelection(ProgramSelection selection, boolean notify) { + protected void setSelection(ProgramSelection selection, boolean notify) { currentSelection = selection; if (selection == null) { return; diff --git a/Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java b/Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java index 794d4cdd1a..6cd27b4bba 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/WindowActionManager.java @@ -16,11 +16,15 @@ package docking; import java.util.*; +import java.util.Map.Entry; import javax.swing.JMenuBar; import docking.action.DockingActionIf; import docking.menu.*; +import generic.concurrent.ReentryGuard; +import generic.concurrent.ReentryGuard.Guarded; +import ghidra.util.Msg; public class WindowActionManager { private Map actionToProxyMap; @@ -30,6 +34,24 @@ public class WindowActionManager { private boolean disposed; + /** + * Some actions' {@link DockingActionIf#isEnabledForContext(ActionContext)} methods may + * inadvertently display an error dialog, allowing for the Swing thread to reenter and modify + * the {@link #actionToProxyMap}. We want to allow this, but detect it and bail early. + */ + private ReentryGuard reentryGuard = new ReentryGuard<>() { + @Override + protected Throwable violated(boolean nested, Throwable previous) { + if (previous != null) { + return previous; + } + Throwable t = new Throwable(); + Msg.error(WindowActionManager.this, + "Modified action list during context change update", t); + return t; + } + }; + WindowActionManager(WindowNode node, MenuHandler menuBarHandler, DockingWindowManager winMgr, MenuGroupMap menuGroupMap) { @@ -40,6 +62,7 @@ public class WindowActionManager { } void setActions(List actionList) { + reentryGuard.checkAccess(); menuBarMgr.clearActions(); toolBarMgr.clearActions(); actionToProxyMap.clear(); @@ -49,6 +72,7 @@ public class WindowActionManager { } void addAction(DockingActionIf action) { + reentryGuard.checkAccess(); if (action.getMenuBarData() != null || action.getToolBarData() != null) { DockingActionProxy proxyAction = new DockingActionProxy(action); actionToProxyMap.put(action, proxyAction); @@ -58,6 +82,7 @@ public class WindowActionManager { } void removeAction(DockingActionIf action) { + reentryGuard.checkAccess(); DockingActionProxy proxyAction = actionToProxyMap.remove(action); if (proxyAction != null) { menuBarMgr.removeAction(proxyAction); @@ -80,6 +105,7 @@ public class WindowActionManager { } void dispose() { + reentryGuard.checkAccess(); disposed = true; node.setToolBar(null); node.setMenuBar(null); @@ -90,28 +116,42 @@ public class WindowActionManager { void contextChanged(Map, ActionContext> defaultContextMap, ActionContext localContext, Set excluded) { - - if (!node.isVisible() || disposed) { - return; - } - - // Update actions - make a copy so that we don't get concurrent modification - // exceptions during reentrant operations - List list = new ArrayList<>(actionToProxyMap.keySet()); - for (DockingActionIf action : list) { - if (excluded.contains(action)) { - continue; + /** + * We need the guard against reentrant changes to the actionToProxyMap, lest the iterator + * throw a ConcurrentModificationException. If the guard finds a violation, i.e., the map + * has changed, we just bail. Whatever changed the map should also trigger an update to + * context, and so we should have a fresh go here with the new map. + * + * There are three points where there could be reentrant modifications. Those are 1) when + * computing an action's context, 2) when checking if the action is enabled for that + * context, and 3) when setting the proxy's enabled state. There are minimal performance + * trade-offs to consider when deciding which points to check. Critically, we must check + * somewhere between the last point and the end of the loop, i.e., before stepping the + * iterator. We opt to check only that last point, since reentry is uncommon here. + */ + try (Guarded guarded = reentryGuard.enter()) { + if (!node.isVisible() || disposed) { + return; } - DockingActionIf proxyAction = actionToProxyMap.get(action); - ActionContext context = - getContextForAction(action, localContext, defaultContextMap); + for (Entry ent : actionToProxyMap.entrySet()) { + DockingActionIf action = ent.getKey(); + DockingActionIf proxyAction = ent.getValue(); + if (excluded.contains(action)) { + continue; + } - if (context != null) { - proxyAction.setEnabled(proxyAction.isEnabledForContext(context)); - } - else { - proxyAction.setEnabled(false); + // Reentry point 1 + ActionContext context = + getContextForAction(action, localContext, defaultContextMap); + // Reentry point 2 + boolean enabled = + context == null ? false : proxyAction.isEnabledForContext(context); + // Reentry point 3, which we check + proxyAction.setEnabled(enabled); + if (reentryGuard.isViolated()) { + break; + } } } } @@ -134,7 +174,8 @@ public class WindowActionManager { /** * Returns the set of actions for this window. * - *

Note this returns the the original passed-in actions and not the proxy actions that the + *

+ * Note this returns the the original passed-in actions and not the proxy actions that the * window uses. * * @return the set of actions for this window diff --git a/Ghidra/Framework/Generic/src/main/java/generic/concurrent/ReentryGuard.java b/Ghidra/Framework/Generic/src/main/java/generic/concurrent/ReentryGuard.java new file mode 100644 index 0000000000..b301ef1fd8 --- /dev/null +++ b/Ghidra/Framework/Generic/src/main/java/generic/concurrent/ReentryGuard.java @@ -0,0 +1,185 @@ +/* ### + * IP: GHIDRA + * + * 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. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package generic.concurrent; + +/** + * A means of detecting and handling reentrant conditions. + * + *

+ * One example where this has been applied deals with updating actions upon changes in context. If, + * in the course of determining which actions are enabled, one of the {@code isEnabled} methods + * displays an error dialog, the Swing thread reenters its main loop while that dialog is showing, + * but before {@code isEnabled} has returned. This can cause all sorts of unexpected behaviors. + * Namely, a timer could fire, context could change again, etc., and the list of actions being + * updated may also change. At worst, this could result in many exceptions being thrown, because a + * data structure has been modified concurrently. At best, if the loop is allowed to finish, there's + * a lot of wasted time updating actions that will never be displayed. + * + *

+ * In that example, the loop that updates the actions would be the "guarded block." Any point at + * which the list of actions is modified might result in "reentrant access" and should be checked. + * + *

+ * This class provides a primitive for instrumenting, detecting, and properly reacting to such + * conditions. For example, if the modification should not be allowed at all, the guard can throw an + * exception at the moment of reentrant access. Alternatively, if the modification should be + * allowed, the guard would simply set a flag, then the guarded block can check that flag and + * terminate early. + * + *

+ * This implementation is not thread safe. It is designed to check for reentrant access, + * not concurrent access. The client must ensure that only one thread enters the guarded block or + * calls {@link #checkAccess()} at a time. Otherwise, the behavior is undefined. + * + * 

+ * public class ActionManager {
+ * 	private final ReentryGuard<Throwable> reentryGuard = new ReentryGuard<>() {
+ * 		@Override
+ * 		public Throwable violated(boolean nested, Throwable previous) {
+ * 			if (previous != null) {
+ * 				return previous;
+ * 			}
+ * 			return new Throwable(); // record the stack of the violation
+ * 		}
+ * 	};
+ * 	private final List<Action> actions;
+ * 
+ * 	public void addAction(Action action) {
+ * 		// Notify the guard we've committed some reentrant behavior.
+ * 		// Would need to add this to anything that modifies the action list. 
+ * 		reentryGuard.checkAccess();
+ * 		actions.add(action);
+ * 	}
+ * 
+ * 	public void updateActions(Context ctx) {
+ * 		try (Guarded guarded = reentryGuard.enter()) {
+ * 			// There is no need to create a copy, since we'll bail before the next iteration
+ * 			for (Action action : actions) {
+ * 				boolean enabled = action.isEnabledForContext(ctx);
+ * 				if (reentryGuard.getViolation() != null) {
+ * 					break; // Actions has been modified. Bail.
+ * 					// NOTE: This leaves the update incomplete.
+ * 					// Something has to call updateActions again.
+ * 				}
+ * 				actions.setEnabled(enabled);
+ * 			}
+ * 		}
+ * 	}
+ * }
+ *
+ * + * @param the type used to record information about a violation. It cannot be {@link Void}. + */ +public abstract class ReentryGuard { + private final Guarded guarded = new Guarded(this); + private boolean inGuarded; + private T violation; + + public static class Guarded implements AutoCloseable { + private ReentryGuard guard; + + public Guarded(ReentryGuard guard) { + this.guard = guard; + } + + @Override + public void close() { + guard.inGuarded = false; + } + } + + /** + * Notify the guard of entry into the guarded block + * + *

+ * This should always be used in a {@code try-with-resources} block. This will ensure that the + * guard is notified of exit from the guarded block, even in exceptional circumstances. + * + *

+ * NOTE: Re-entering the guarded portion is itself a violation. + * + * @return a closeable for notifying the guard of exit from the guarded block, or null if + * reentering the guarded block + */ + public Guarded enter() { + if (inGuarded) { + violation = violated(true, violation); + return null; + } + inGuarded = true; + violation = null; + return guarded; + } + + /** + * Notify the guard of access to some resource used by the guarded block + * + *

+ * If the access turns out to be reentrant, i.e., the thread's current stack includes a frame in + * the guarded block, this will call {@link #violated(boolean, Object)} and record the result. + * It can be inspected later via {@link #getViolation()}. + */ + public void checkAccess() { + if (inGuarded) { + violation = violated(false, violation); + } + } + + /** + * Record a violation + * + *

+ * This method is called if {@link #checkAccess()} or {@link #enter()} is called while already + * inside a guarded block. Its return value is stored for later inspection by + * {@link #getViolation()}. It's possible multiple violations occur within one execution of the + * guarded block. The previous return value of this method is provided, if that is the case. To + * record only the first violation, this method should just {@code return previous} when it is + * non-null. To record only the last violation, this method should disregard {@code previous}. + * To record all violations, {@code T} will need to be a collection, and this method will need + * to create and/or append to the collection. + * + * @param nested true if the violation is a nested call to {@link #enter()}; false if the + * violation is a call to {@link #checkAccess()} + * @param previous the previous return value of this method, on the occasion of multiple + * violations + * @return the record of the violation + */ + protected abstract T violated(boolean nested, T previous); + + /** + * Retrieve a violation, if applicable + * + *

+ * Calling this method outside of a guarded block has undefined behavior. + * + * @return the violation; or null to indicate no violation + */ + public T getViolation() { + return violation; + } + + /** + * Check if there is a violation + * + *

+ * This is equivalent to checking if {@link #getViolation()} returns non-null. + * + * @return true if there is a violation. + */ + public boolean isViolated() { + return violation != null; + } +}