Decompiler Action Blocked

[astrelsky]

Describe the bug
Attempting to use any action in the decompiler while the decompiler is running results in a pop-up with the title "Decompiler Action Blocked" and message "You cannot perform Decompiler actions while the Decompiler is busy". If the Decompiler is busy, the action should not be enabled. The current behavior contradicts the design of DockingAction

To Reproduce
Steps to reproduce the behavior:

1. Find a huge function that takes forever to decompile.
2. When it is finished, rename a local variable and then try to rename it again while it is decompiling.
3. Select any action from the choice dialog that also shouldn't be there.
4. See message.

Expected behavior
If the Decompiler is busy, the actions should be disabled.

Environment (please complete the following information):

• Ghidra Version: All publicly available versions.

Additional context
I want to be able to spam the keys to rename and retype until it works because I am impatient.

[dragonmacher]

The subclasses AbstractDecompilerAction is designed to wait for the Decompiler, with the understanding that those actions need the decompiled results to run. It is possible that not all extensions of this action need the results. If the results are needed, not sure we can change anything here. The requests could be buffered, but that would at 2.43 metric tonnes of complexity.

[astrelsky]

The subclasses AbstractDecompilerAction is designed to wait for the Decompiler, with the understanding that those actions need the decompiled results to run. It is possible that not all extensions of this action need the results. If the results are needed, not sure we can change anything here. The requests could be buffered, but that would at 2.43 metric tonnes of complexity.

Right. But while it is waiting on the Decompiler, those actions should not be enabled. I should not be able to attempt to rename a local variable while the function is decompiling if I'm not allowed to do so. Attempting to access the action via right click or keyboard shortcut should do nothing instead of leading to a set of conflicting options that all result in the aforementioned pop-up dialogue.

[dragonmacher]

I see now. I missed our original point.

We can look into changing that. I don't recall now why I did it that way, but I suspect there was a technical limitation.

[dragonmacher]

Here is the comment that addresses the 'why' it was done this way:

		//
		// Unusual Code: actions will call this method when their 'isEnabledForContext()' is
		//               called.  If the decompiler is still working, we return true here so
		//               the action is considered enabled.  This allows any key bindings registered
		//               for the action to get consumed.  If we did not return false when
		//               the decompiler was still working, then the key binding would not match and
		//               the system would pass the key binding up to the global action system,
		//               which we do not want.
		//
		//               Each action that needs state from the decompiler must call this method   
		//               from 'isEnabledForContext()'.  Also, each action must call 
		//               'performAction()' on this class, which will skip the action's work and
		//               show an message if the decompiler is busy.
		//

---

For us to make this work, we would need a way to tell the framework that we have actions that should consume the the given keystroke, but that happen to not be enabled at this time. Currently, we use the enablement to know when to let the global actions process a given keybindings.

[astrelsky]

Wouldn't the global actions not consume it either? The action context would be within the Decompiler which nothing else is going to consume so it should behave the same as if I pressed an unbound key.

[dragonmacher]

Yes, you could update your bindings to avoid this issue. But, the default actions would still potentially exhibit the issue. Plus, users would have to know to avoid reusing bindings if they run into this. It feels safer to me to add a mechanism that allows the framework to know how to avoid the issue.

[astrelsky]

Yes, you could update your bindings to avoid this issue. But, the default actions would still potentially exhibit the issue. Plus, users would have to know to avoid reusing bindings if they run into this. It feels safer to me to add a mechanism that allows the framework to know how to avoid the issue.

You would not typically have conflicting key bindings. Using this logic, this dialogue should appear for any key press in the decompiler window when it is busy, regardless of whether or not it has a key binding.

The only time conflicting actions are seen are for things like rename function and rename label, in which case the action does the same thing and the choice dialog is just an annoyance.

I'd also argue that it could be acceptable to punt with a custom unchecked exception here to signal that it should be consumed but no action taken. This is assuming you would need to go much further back in the event handling to be able to consume it and do nothing.