Fix crash on exit when closing multiple profiles#8203
Fix crash on exit when closing multiple profiles#8203vadi2 merged 20 commits intoMudlet:developmentfrom
Conversation
|
Please review this PR ad let me know what you think by dropping a comment @vadi2 @ZookaOnGit @Kebap |
|
Closed it by mistake |
|
@Excellencedev please provide a screencast where this is shown to work, thank you: 
|
|
Will do so |
|
@vadi2 Here is the screencast: https://github.com/user-attachments/assets/526aa1eb-e246-4977-ba49-d4fbc34483d7 I opened 16 profiles on Mudlet, and closed them quickly with Alt + W and there was no crash |
|
The screencast doesn't seem to show profiles being closed at all, did you attach the right video? |
|
@vadi2 I am sorry for the mistake. I posted the wrong video |
|
@vadi2 Here is the correct screencast: https://github.com/user-attachments/assets/49c0c926-1e97-4000-aceb-f47511e6ab51 |
|
Hello @vadi2, have you checked the new screencast: https://github.com/user-attachments/assets/49c0c926-1e97-4000-aceb-f47511e6ab51 and what do you think |
|
Hello @vadi2 , I sent a new screencast |
|
It's been a week. Hello @vadi2 |
|
Still crashes unfortunately when I run the test described. Here is one stacktrace (now different from what was there before): What OS are you testing this on? |
Windows |
…ust across Windows/Linux).
|
@vadi2 I might have fixed it now The 1st fix from before (mpLastCommandLineUsed.clear()) worked because Qt on Windows doesn’t deliver events during destruction. On Linux, however, Qt continues processing pending events, which caused the crash. The stacktrace confirms this: a QKeyEvent was still delivered to TCommandLine::keybindingMatched(), which called into KeyUnit/Tree objects that had already been destroyed → classic use-after-free. The new commit adds a guard in TCommandLine::keybindingMatched() to skip processing when the host is shutting down, making the fix robust across platforms. Crash site: #0 Tree::isActive() const → This is being called from inside TKey::match(). That means something is calling into a keybinding structure that’s already been torn down. Classic use-after-free. Call path: TCommandLine::keybindingMatched(QKeyEvent*) → This shows a QKeyEvent being delivered to TCommandLine while destruction is already happening. Qt event loop evidence: QApplicationPrivate::notify_helper → Confirms that Qt’s event system was still dispatching key events after the underlying structures were already being destroyed. |
|
Using the latest commit, the stacktrace looks the same: Try installing Linux in Virtualbox, both are free, and you'll be able to see the same result there. |
|
I think it is fixed rn |
|
Hello @vadi2 ? |
vadi2
left a comment
There was a problem hiding this comment.
Nice! No crashes now. But it adds C++ exceptions, which you might notice we don't use in the codebase. Try fixing the issue without adding them.
|
I will do that @vadi2. Thanks for review |
…tdown by replacing the `try-catch` block with checks for `keyObject->isActive()` and `keyObject->mpHost->isClosingDown()`. Modified `src/TCommandLine.cpp` to remove exception handling and added null checks for `mpHost` to prevent crashes during profile shutdown.
|
@vadi2 I have removed the try catch blocks. it still does not crash |
|
@vadi2 I have removed the try catch blocks. it still does not crash. Hello ? |
|
This is a volunteer project that we work on at our free time, don't spam us please. |
@vadi2 So sorry |
|
@vadi2 Thanks |
/claim #7478
/closes #7478
Description:
This PR resolves a crash that occurs when closing multiple profiles simultaneously.
The root cause was a race condition in the Host destructor, where weak pointers to command line objects (mActiveCommandLines) were being accessed after they were no longer valid.
The fix involves explicitly clearing the mActiveCommandLines container at the beginning of the Host destructor. This ensures that the resources are released in a safe order, preventing the crash.
Here is the correct screencast @vadi2 : https://github.com/user-attachments/assets/49c0c926-1e97-4000-aceb-f47511e6ab51