Bug 466062

Summary:	Invalid write after KXMLGUIFactory::removeClient when menu not fully closed
Product:	[Frameworks and Libraries] frameworks-kxmlgui	Reporter:	wojnilowicz <lukasz.wojnilowicz>
Component:	general	Assignee:	kdelibs bugs <kdelibs-bugs-null>
Status:	RESOLVED DUPLICATE
Severity:	normal	CC:	aacid, christoph, maris.kde
Priority:	NOR	Keywords:	testcase
Version First Reported In:	5.103.0
Target Milestone:	---
Platform:	Fedora RPMs
OS:	Linux
See Also:	https://bugs.kde.org/show_bug.cgi?id=460634
Latest Commit:		Version Fixed/Implemented In:
Sentry Crash Report:
Attachments:	Test case for bug #460634 valgrind output while clicking on find another valgrind run while clicking on find in lokalize gdb output while clicking on find and then closing window

Description wojnilowicz 2023-02-19 09:25:57 UTC

Created attachment 156478 [details]
Test case for bug #460634

SUMMARY
In the attached test case, triggering Edit->Find with a mouse causes a memory leak. Triggering it with a keybord shortcut Ctrl+F doesn't cause a memory leak. The same mechanism is used in Lokalize and eventually causes there a crash.

I figured out that the crash happens because QEvent::MouseMove on a menu widget, being closed after click operation, gets converted to QEvent::MouseButtonRelease in QWidgetWindow::handleMouseEvent and is being sent to the main window/menu widget somewhere in the process of deleting menu widget, but reaches the menu widget after its deletinon.

It's difficult for me to decide on which side the bug is: Qt, KDE or Lokalize. Qt sends an event to a widget it knows that is in the process of deleting. KDE in KXMLGUIFactory::removeClient uses a delete operator instead of deleteLater method for widgets to be deleted and not checking if there is already any signal in the event queue for them.

STEPS TO REPRODUCE
1. Compile the attached test case
2. Execute "valgrind --tool=memcheck myapp" in terminal
3. Click on Edit->Find with mouse (important to do it with a mouse!)

OBSERVED RESULT
"Invalid write of size 1" in terminal.

EXPECTED RESULT
No "Invalid write of size 1" in terminal.

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Fedora 37
KDE Plasma Version: 5.27.0
KDE Frameworks Version: 5.103.0
Qt Version: 5.15.8

ADDITIONAL INFORMATION

Comment 1 Albert Astals Cid 2023-02-19 22:58:56 UTC

This is not a memory leak.

A memory leak is when you forget to delete memory, there's no memory being forgotten to be deleted here.

In my opinion this is a bug of the application, it is removing things under Qt's feet, so Qt crashes

Comment 2 wojnilowicz 2023-02-21 16:10:11 UTC

Ok, I corrected bug's subject, thanks for the clarification.

In general case all can be pointed to the application. On the other hand if the application would have to create menus itself then it would probably use deleteLater on them but instead it delegates this task to KXmlGuiWindow which by description at
https://api.kde.org/frameworks/kxmlgui/html/classKXmlGuiWindow.html#details
should serve this purpose.

Comment 3 Māris Nartišs 2024-01-17 11:27:20 UTC

In the mean time it causes easy to reproduce crashes in Lokalize (I lost unsaved changes when decided to use search functionality in Lokalize :-(

Comment 4 Albert Astals Cid 2024-01-25 21:28:10 UTC

Māris  which Lokalize version are you using and what's the backtrace of the crash?

Comment 5 Māris Nartišs 2024-01-26 14:04:12 UTC

Created attachment 165240 [details]
valgrind output while clicking on find

It can take a few times to click Edit->Find then close search and replace tab (usually errors on the second "find").

Comment 6 Māris Nartišs 2024-01-26 14:05:20 UTC

Created attachment 165241 [details]
another valgrind run while clicking on find in lokalize

Comment 7 Māris Nartišs 2024-01-26 14:11:14 UTC

Created attachment 165243 [details]
gdb output while clicking on find and then closing window

gdb turned out to be less helpful as backtrace leads to different sources indicating on a potential prior memory corruption. Still, usually it just takes "Edit"->"Find"->close search tab->"Edit"->"Find" to trigger the crash.

Debian sid
Lokalize Version 22.12.3
KDE Frameworks Version 5.107.0
Qt Version 5.15.10 (built against 5.15.10)
The xcb windowing system

Comment 8 wojnilowicz 2024-01-26 15:09:57 UTC

Your issue is already solved in bug #460634 and you're using Lokalize version not having the fix from that bug.

This bug is about another issue.

Comment 9 Māris Nartišs 2024-01-26 16:38:21 UTC

(In reply to NSLW from comment #8)
> Your issue is already solved in bug #460634 and you're using Lokalize
> version not having the fix from that bug.
> 
> This bug is about another issue.

Sorry for noise. I forgot that I have switched from Gentoo to Debian Sid (unstable) and thus ancient packages is a norm even when running the bleeding edge version :-(

Comment 10 Christoph Cullmann 2025-01-19 19:16:30 UTC

Yeah, that is an app bug and that got fixed there, deleting a menu or widget during it's event handling is no good idea.

*** This bug has been marked as a duplicate of bug 460634 ***