Refactor CutterCore into RizinWrapper and Sessions

[PremadeS]

Your checklist for this pull request

• I've read the guidelines for contributing to this repository
• I made sure to follow the project's coding style
• I've updated the documentation with the relevant information (if needed)
• I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

This is going to be a long one, so grab some popcorn 🍿

Cutter currently supports opening a single binary in one application instance. In order to open another binary, a separate cutter app instance has to be opened. Because multiple sessions aren't supported, all communication with rizin happens through a singleton class (CutterCore) which maintains a single raw rizin core (RzCore) and a "recursive" mutex protecting it along with a lot of other GUI stuff.

If we want to open multiple binaries - say for diffing, we need two additional RzCore objects. The issue is in order to display them in the same widgets (DisassemblyWidget, Graph etc) we would have to duplicate all of the logic for those widgets because every widget class inside Cutter is hardcoded to query the singleton CutterCore instance.

Aside from diffing, adding support for opening multiple binaries in the same app instance with the current implementation is not possible.

Solution:

This PR is first half of a bigger refactor

1- What this PR does:

i) Separate CutterCore into RizinWrapper and Sessions

• RizinWrapper: Owns a single RzCore and contains methods that convert rizin C data structures into equivalent C++ for cutter, it doesn't hold any mutexes but does emit signals. This emition of signals inside the wrapper can be modified once rizin implements it's own mechanism of signals (rzevents etc)
• CoreSession: Contains logic related to GUI, such as communication between widgets, (triggerAsmOptionsChanged(), showMemoryWidgetRequested() etc), and manages AsyncTaskManager. It also owns a single RizinWrapper instance and a mutex for it. The mutex is kept recursive for now (see "Note" at the end). If any widget needs to query rizin it does so by calling the wrapper function through the scoped lock provided by a helper method inside CoreSession. Directly accessing the wrapper without lock should be avoided.
• DynamicSession: Inherits from "CoreSession" and contains extra logic required for debugging. The reason for separating this from "CoreSession" will be apparent in (2)(i). To avoid such a big refactor in one single PR, DynamicSession instance is kept singleton and Core() macro is updated to provide a temporary lock in order to access the wrapper functions, This keeps most of the Core()->func() logic same for now. Next step would be to remove the singleton all together and pass a CoreSession pointer to every widget which they can query for data

Note that the names for classes are not final, these just made sense to me.

ii) All of the global logic (like registering a Decompiler etc) is handled by the CutterApplication class

iii) The single DynamicSession is managed by the MainWindow. When the support for multiple sessions is added, it might be better to create sort of a "session manager" which would manage currently opened sessions

2- How does this help the diffing issue?

i) Create a "DiffSession" class which will manage/own (not inherit) two "CoreSession" objects, one for eacah binary. Both "CoreSession" objects are separate from each other, so analysis can be run for both in separate threads. Since diffing does not require any of the debuging logic, hence why "DynamicSession" is kept separate from "CoreSession"

ii) When Part (2) of this refactor is done (removing the singleton), pass those sessions to which ever widget that needs to be rendered. If additional logic is required or some logic needs to be replaced then create a single base class containing the same code and inherit two widgets from it, one for the MainWindow and one for DiffWindow.

iii) It might be better to keep the DiffWindow completely separate from the MainWindow and let CutterApp manage both. There will be an action in MainWindow that will emit a signal indicating CutterApplication to open the DiffWindow. DiffWindow will contain logic for the window itself which will contain each of the widgets for diffing. Keeping the DiffWindow separate from MainWindow will also give the option to open diff window from the initial dialog, completely avoiding the MainWindow itself.

Note: From the text above it is apparent that locking is handled outside of the wrapper, compared to previous approach which locked a mutex at the start of every CutterCore function. The issue with this and the previous approach is that almost all functions are called on the main UI thread and whenever a signal is emitted the equivalent slots are immediately called, during all of this the mutex is kept locked (as it in scope) - meaning if any function is to be called in a background thread it has to wait for all of the UI updates to finish. This can be mostly fixed by implementing the "Event-Driven approach" - by allowing every rizin call to be in a separate thread to UI. Since the signals will be emitted in a background thread and slot will be in the main UI thread, the connection type will be "QueuedConnection" and the slot will only run once the control passes back to the main thread. When the "rizin background thread approach" is implemented we can potentially drop the recursive mutex and switch to a read-write lock but the exact details will be cleaer when implementing

Test plan (required)

• All CI passes
• Everything works as it should

Closing issues

[PremadeS]

Note that this is still a draft PR, I need to still fix some things and review it one more time. There are also some TODO's that I've probably missed.

[PremadeS]

cc: @NewtronReal

[Rot127]

In general that seems to me like a good refactor.
Though, there is a "but":

The real problem is that the Rizin core is not thread safe, right?
So I wonder if all the energy, which has to go into the refactor, is maybe better spent on making Rizin thread safe. And afterwards refactor Cutter properly.

I am not sure how much is actually missing on the Rizin side for that.
Definitely rizinorg/rizin#4055 and RzCons. RzIO as well I think.
But @wargio @thestr4ng3r @notxvilka should know best.

If "Rizin thread safe" project will continue to take forever this here is sensible.

[PremadeS]

So I wonder if all the energy, which has to go into the refactor, is maybe better spent on making Rizin thread safe. And afterwards refactor Cutter properly.

I agree with this, it would also completely avoid the signal/slot issue I mentioned where the lock is held longer than needed.

However CutterCore would still have to be split into sessions and wrapper to make widgets reusable and to avoid the "God Object" pattern

[thestr4ng3r]

The issue with this and the previous approach is that almost all functions are called on the main UI thread and whenever a signal is emitted the equivalent slots are immediately called, during all of this the mutex is kept locked (as it in scope)

There are two other possibilities to solve this problem though, with slightly different semantics:

1. The scope of the lock can be narrowed locally:

   {
   	RzCoreLocked core(Core());
   	// do something with core
   }
   emit someSignal();
   

2. The connection type can be passed as Qt::QueuedConnection explicitly to connect https://doc.qt.io/qt-6/qobject.html#connect-5

If "Rizin thread safe" project will continue to take forever

This. I don't think it is neither realistic nor sensible to make all of rizin itself thread safe. Some lower subsystems like IO should be but for the high-level core a single thread or event-loop-like approach like we do is likely the most fitting. It is also conceptually similar to how most modern UI frameworks work where there is a main event loop on one thread and parallel background computations like the actual rendering.