Crash if you edit draft label and minimize the window

A forum dedicated to the Draft, Arch and BIM workbenches development.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
hko
Posts: 96
Joined: Thu Apr 23, 2020 10:44 pm

Crash if you edit draft label and minimize the window

Post by hko »

  1. Create draft label with Custom label type.
  2. Edit Custom Text property
  3. Minimize the opened "List" editor window
  4. Observe that FreeCAD crashes. Stacktrace below.
I wouldn't explicitly minimize the editor window but I would (and did) change the virtual desktop while editing and got the same result.

Code: Select all

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00001555500bb859 in __GI_abort () at abort.c:79
#2  0x000015555012626e in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x155550250298 "%s\n")
    at ../sysdeps/posix/libc_fatal.c:155
#3  0x000015555012e2fc in malloc_printerr (str=str@entry=0x155550252670 "double free or corruption (out)") at malloc.c:5347
#4  0x000015555012ffa0 in _int_free (av=0x155550285b80 <main_arena>, p=0x7fffffffbbe0, have_lock=<optimized out>) at malloc.c:4314
#5  0x00001555548cba99 in PropertyListDialog::~PropertyListDialog() (this=0x7fffffffbbf0, __in_chrg=<optimized out>)
    at ../src/Gui/Widgets.cpp:1353
#6  0x0000155550737eee in QObjectPrivate::deleteChildren() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#7  0x0000155551166c29 in QWidget::~QWidget() () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#8  0x00001555548c1df2 in Gui::LabelEditor::~LabelEditor() (this=0x55555e090290, __in_chrg=<optimized out>)
    at ../src/Gui/Widgets.cpp:1426
#9  0x00001555548c1e1c in Gui::LabelEditor::~LabelEditor() (this=0x55555e090290, __in_chrg=<optimized out>)
    at ../src/Gui/Widgets.cpp:1428
#10 0x000015555073aa44 in QObject::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#11 0x000015555116b25d in QWidget::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#12 0x0000155551128a66 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#13 0x00001555511320f0 in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#14 0x000015555427b6d0 in Gui::GUIApplication::notify(QObject*, QEvent*) (this=
    0x7fffffffd160, receiver=0x55555e090290, event=0x55555e24c200) at ../src/Gui/GuiApplication.cpp:84
#15 0x000015555070e80a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#16 0x0000155550711488 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
    at /lib/x86_64-linux-gnu/libQt5Core.so.5
#17 0x0000155550766e37 in  () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#18 0x000015554e3bb17d in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#19 0x000015554e3bb400 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#20 0x000015554e3bb4a3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#21 0x0000155550766435 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /lib/x86_64-linux-gnu/libQt5Core.so.5
#22 0x000015555070d3ab in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#23 0x000015555132cc6d in QDialog::exec() () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#24 0x00001555548c2224 in Gui::LabelEditor::changeText() (this=0x55555e090290) at ../src/Gui/Widgets.cpp:1464
#25 0x00001555548c61c0 in Gui::LabelEditor::qt_static_metacall(QObject*, QMetaObject::Call, int, void**)
    (_o=0x55555e090290, _c=QMetaObject::InvokeMetaMethod, _id=4, _a=0x7fffffffbdc0)
    at src/Gui/FreeCADGui_autogen/include/moc_Widgets.cpp:1505
#26 0x000015555073a1d0 in QMetaObject::activate(QObject*, int, int, void**) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#27 0x000015555121d806 in QAbstractButton::clicked(bool) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#28 0x000015555121da2e in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#29 0x000015555121ee73 in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#30 0x000015555121f035 in QAbstractButton::mouseReleaseEvent(QMouseEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#31 0x000015555116b2b6 in QWidget::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#32 0x0000155551128a66 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#33 0x0000155551132343 in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#34 0x000015555427b6d0 in Gui::GUIApplication::notify(QObject*, QEvent*)
    (this=0x7fffffffd160, receiver=0x555555ddab20, event=0x7fffffffc550) at ../src/Gui/GuiApplication.cpp:84
#35 0x000015555070e80a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#36 0x0000155551131457 in QApplicationPrivate::sendMouseEvent(QWidget*, QMouseEvent*, QWidget*, QWidget*, QWidget**, QPointer<QWidget>&, bool, bool) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#37 0x000015555118735d in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#38 0x000015555118a1ec in  () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#39 0x0000155551128a66 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#40 0x00001555511320f0 in QApplication::notify(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#41 0x000015555427b6d0 in Gui::GUIApplication::notify(QObject*, QEvent*)
    (this=0x7fffffffd160, receiver=0x5555574b13e0, event=0x7fffffffcbf0) at ../src/Gui/GuiApplication.cpp:84
#42 0x000015555070e80a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#43 0x0000155550af97d3 in QGuiApplicationPrivate::processMouseEvent(QWindowSystemInterfacePrivate::MouseEvent*) ()
    at /lib/x86_64-linux-gnu/libQt5Gui.so.5
#44 0x0000155550afb10b in QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*) ()
    at /lib/x86_64-linux-gnu/libQt5Gui.so.5
#45 0x0000155550ad535b in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /lib/x86_64-linux-gnu/libQt5Gui.so.5
#46 0x000015554a16332e in  () at /lib/x86_64-linux-gnu/libQt5XcbQpa.so.5
#47 0x000015554e3bb17d in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#48 0x000015554e3bb400 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#49 0x000015554e3bb4a3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#50 0x0000155550766435 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /lib/x86_64-linux-gnu/libQt5Core.so.5
#51 0x000015555070d3ab in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#52 0x0000155550715116 in QCoreApplication::exec() () at /lib/x86_64-linux-gnu/libQt5Core.so.5
#53 0x0000155554127fe2 in Gui::Application::runApplication() () at ../src/Gui/Application.cpp:2256
#54 0x0000555555566acf in main(int, char**) (argc=1, argv=0x7fffffffddf8) at ../src/Main/MainGui.cpp:285

Code: Select all

OS: Ubuntu 20.04.4 LTS (KDE/plasma)
Word size of FreeCAD: 64-bit
Version: 0.20.24436 (Git)
Build type: Debug
Branch: master
Hash: ecbd151b114e086ee72d6142ace5780f6220c08a
Python 3.8.10, Qt 5.12.8, Coin 4.0.0, Vtk 7.1.1, OCC 7.3.0
Locale: English/United States (en_US)
Installed mods: 
  * 3DfindIT 1.2.0
  * Render 2022.2.0
  * BIM 2021.12.0
  * Manipulator 1.4.9
  * parts_library
  * dodo
paullee
Veteran
Posts: 5097
Joined: Wed May 04, 2016 3:58 pm

Re: Crash if you edit draft label and minimize the window

Post by paullee »

hko wrote: Sun Jul 31, 2022 2:31 am
  1. Create draft label with Custom label type.
  2. Edit Custom Text property
  3. Minimize the opened "List" editor window
  4. Observe that FreeCAD crashes. Stacktrace below.
I wouldn't explicitly minimize the editor window but I would (and did) change the virtual desktop while editing and got the same result.
Confirmed, with some difference :)
  • I am on 0.20 release, ApppImage version, on Fedora 36
  • There is no button / contextual menu to minimize the editor window
  • Changing virtual desktop to another one, FC crashes
  • Not knowing where is 'stacktrace', just some message from command window

Code: Select all

[paullee@fedora squashfs-root]$ FreeCAD 0.20, Libs: 0.20R29177 (Git)
© Juergen Riegel, Werner Mayer, Yorik van Havre and others 2001-2022
FreeCAD is free and open-source software licensed under the terms of LGPL2+ license.
FreeCAD wouldn't be possible without FreeCAD community.
  #####                 ####  ###   ####  
  #                    #      # #   #   # 
  #     ##  #### ####  #     #   #  #   # 
  ####  # # #  # #  #  #     #####  #   # 
  #     #   #### ####  #    #     # #   # 
  #     #   #    #     #    #     # #   #  ##  ##  ##
  #     #   #### ####   ### #     # ####   ##  ##  ##

[paullee@fedora squashfs-root]$ flag
flag2
----------------
GuiCommand: Label
Pick target point
Pick endpoint of leader line
Pick text position
----------------
Label
target_point: Vector (0.0, 0.0, 0.0)
placement: Placement [Pos=(109.175,55.392,0), Yaw-Pitch-Roll=(0,0,0)]
label_type: Custom
custom_text: Label
direction: Horizontal
distance: -64.50500106811523
double free or corruption (out)
./AppRun: line 42: 10790 Aborted                 (core dumped) ${MAIN} "$@"

[1]+  Exit 134                ./AppRun
[paullee@fedora squashfs-root]$ 

FreeCAD-0.20.0-Linux-x86_64.AppImage
on
Fedora 36

Code: Select all

OS: Fedora Linux 36 (Workstation Edition) (GNOME/gnome)
Word size of FreeCAD: 64-bit
Version: 0.20.29177 (Git)
Build type: Release
Branch: (HEAD detached at 0.20)
Hash: 68e337670e227889217652ddac593c93b5e8dc94
Python 3.9.13, Qt 5.12.9, Coin 4.0.0, Vtk 9.1.0, OCC 7.5.3
Locale: English/United States (en_US)
Installed mods: 
  * dodo
  * DynamicData 2.46.0
  * ArchTextures

Screenshot from 2022-07-31 10-44-30.png
Screenshot from 2022-07-31 10-44-30.png (7.15 KiB) Viewed 1638 times
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

Re: Crash if you edit draft label and minimize the window

Post by yorik »

I also have no way to minimize the editor window, but indeed switching virtual desktops also causes a crash.
The problem is not in Draft Label though, any other object that uses the PropertyStringList editor has the same issue. The problem seems to be when the dialog is being closed (I don't know why it is closed when switching desktops, though) - PropertyListDialog::~PropertyListDialog()
hko
Posts: 96
Joined: Thu Apr 23, 2020 10:44 pm

Re: Crash if you edit draft label and minimize the window

Post by hko »

The dialog getting closed is not the only unwanted side effect of switching the virtual desktop or minimizing FreeCAD: if you expand nodes in the property editor (for example, open Placement tree and then Position tree) and then switch to another virtual desktop and back again, the nodes you just expanded are unexpanded again.

I guess it depends on the OS / windowing system but at least in X11/KDE leaving a virtual desktop with a window seems to be quite equivalent to minimizing the window in terms of events the window gets (can be quickly checked with for example xev -id window_id).
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Crash if you edit draft label and minimize the window

Post by openBrain »

OK, again I had to let that cool down but finally I caught it. :)

The problem is here : https://github.com/FreeCAD/FreeCAD/blob ... .cpp#L1444
and here : https://github.com/FreeCAD/FreeCAD/blob ... .cpp#L1457

As for the global picture of the problem, Qt tells it better than me. Just read the note here :) : https://doc.qt.io/qt-6/qdialog.html#exec

With details :
* The first line I linked above create the PropertyListDialog with its parent being the LabelEditor
* The second line opens it in a modal way waiting for user to complete it. As underlined by Qt, the problem of 'exec()' is that it runs the dialog in a separate event loop
* The property pane uses a mechanism called "persistent editor". Roughly, lines is the property pane are "read only", and only when one gets focus (mouse click or Tab or ...) it changes to present to user the persistent editor to allow value editing. In the present case, LabelEditor is the name of the persistent editor used for value list editing.
* The persistent editor itself has an internal mechanism that, when it lost focus, it is closed and value comes back to "read only" -- which seems logical behavior --

Now what happens :
* In first GH line above, a PropertyListDialog is instanciated and displayed modal (using another event loop) with the 2nd GH line
* While dialog is modal, user is still allowed to minimize the main window. When user do so, the original (main window) event loop processes a FocusOut to the persistent editor, which deletes the LabelEditor and so automatically its children (which PropertyListDialog is)
* This automatically rejects the PropertyListDialog so 'exec()' returns (at the 2nd GH line)
* Function ends and PropertyListDialog instance (created at 1st GH line) comes out of scope so program want to delete it but it has already been deleted at the step above => SEGFAULT and crash

@wmayer I see 3 possibilities to solve. Would like your opinion before starting something. From the laziest to the bravest. :)

1) In the 1st GH line pointed above, initialized 'dlg' as a pointer (with 'new') then never delete the dialog ourselves directly and only rely on Qt children deletion to do it. => A consequence is that if user minimizes the main window while the dialog open, it is automatically closed and lost (it won't come back when user will restore the window). Also obviously the pointer cannot be used after 'exec()' because it may be dangling.

2) In the 1st GH line, use the main window as parent for the PropertyListDialog. This ensures safe ownership, the drawback being that if some magics injects a FocusOut event to the persistent editor in the main event loop, the dialog won't be deleted. Though this is very unlikely to happen. => A consequence is that if the user minimizes the main window, the dialog will be preserved and restored when user will restore the main window.

3) Use 'open()' to open the dialog and connect to its 'accepted' signal to process data when returning. This is the "proper" way, but will induce that dialog is automatically closed and lost (as in solution 1) when user minimizes the main window. Not sure if it can be perceived as being not very user friendly (let say one is minimizing to check some related data on another app). Of course 2 and 3 can be mixed so it doesn't happen.

Thanks for insights.

For the record : PropertyListDialog isn't the only class with this issue. VectorListEditor got it as well, other classes used as persistent editor shall be checked.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Crash if you edit draft label and minimize the window

Post by wmayer »

openBrain wrote: Fri Oct 21, 2022 1:53 pm Now what happens :
But this is not the full story. If you have version v0.18 around none of the observed problems occur. This means minimizing the main window while the modal dialog is open doesn't cause the property editor to close its active editor and thus doesn't destroy the dialog that has been created on the stack.

Not even the problem of the collapsed sub-items occur when switching to another virtual desktop or minimizing the main window.

The problems have been introduced by the heavy changes made by RT since 2019 but it's difficult to say which commit exactly is to blame. At least the problem already occurs in v0.19. Another commit I remember was git commit 75dd3d6533c6 that has caused many similar problems as described here, e.g. https://forum.freecadweb.org/viewtopic.php?f=35&t=67083
1) In the 1st GH line pointed above, initialized 'dlg' as a pointer (with 'new') then never delete the dialog ourselves directly and only rely on Qt children deletion to do it. => A consequence is that if user minimizes the main window while the dialog open, it is automatically closed and lost (it won't come back when user will restore the window). Also obviously the pointer cannot be used after 'exec()' because it may be dangling.
What might work is to use a QPointer and creating the dialog on the heap. After it returns from exec() one can check if the QPointer is null.
2) In the 1st GH line, use the main window as parent for the PropertyListDialog. This ensures safe ownership, the drawback being that if some magics injects a FocusOut event to the persistent editor in the main event loop, the dialog won't be deleted. Though this is very unlikely to happen. => A consequence is that if the user minimizes the main window, the dialog will be preserved and restored when user will restore the main window.
This apparently doesn't work. We had a very similar case in VectorListWidget::buttonClicked() where in the past the MainWindow was the parent. But this led to this crash https://forum.freecadweb.org/viewtopic.php?f=10&t=66992 and the fix was to replace the parent with "this". So, this "fix" must be double-checked now whether it still works.
3) Use 'open()' to open the dialog and connect to its 'accepted' signal to process data when returning. This is the "proper" way, but will induce that dialog is automatically closed and lost (as in solution 1) when user minimizes the main window. Not sure if it can be perceived as being not very user friendly (let say one is minimizing to check some related data on another app). Of course 2 and 3 can be mixed so it doesn't happen.
Yes, that's the cleanest solution.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Crash if you edit draft label and minimize the window

Post by wmayer »

wmayer wrote: Sat Oct 22, 2022 3:21 pm This apparently doesn't work. We had a very similar case in VectorListWidget::buttonClicked() where in the past the MainWindow was the parent. But this led to this crash https://forum.freecadweb.org/viewtopic.php?f=10&t=66992 and the fix was to replace the parent with "this". So, this "fix" must be double-checked now whether it still works.
As I already assumed this "fix" suffers from the exact same problem. To reproduce the problem create a Draft wire and edit the Points property. If you minimize the main window FreeCAD crashes.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Crash if you edit draft label and minimize the window

Post by wmayer »

wmayer wrote: Sat Oct 22, 2022 3:21 pm What might work is to use a QPointer and creating the dialog on the heap. After it returns from exec() one can check if the QPointer is null.
Although this works it's not a nice solution.
3) Use 'open()' to open the dialog and connect to its 'accepted' signal to process data when returning. This is the "proper" way, but will induce that dialog is automatically closed and lost (as in solution 1) when user minimizes the main window. Not sure if it can be perceived as being not very user friendly (let say one is minimizing to check some related data on another app). Of course 2 and 3 can be mixed so it doesn't happen.
What's nice is that we can use the overloaded connect() function that accepts a lambda expression that allows us to write the entire code inside a single function. But when doing so we have to use exec() instead of open()
openBrain
Veteran
Posts: 9034
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Crash if you edit draft label and minimize the window

Post by openBrain »

wmayer wrote: Sat Oct 22, 2022 3:29 pm As I already assumed this "fix" suffers from the exact same problem. To reproduce the problem create a Draft wire and edit the Points property. If you minimize the main window FreeCAD crashes.
Yep. I did a very quick test but I think I then only rejected the dialog, so no further code tried to access the widgets. From the code, it's pretty that it crashes here too if the dialog is accepted. Silly me. :lol:
wmayer wrote: Sat Oct 22, 2022 4:31 pm Although this works it's not a nice solution.
Can only agree on that.
wmayer wrote: Sat Oct 22, 2022 4:31 pm What's nice is that we can use the overloaded connect() function that accepts a lambda expression that allows us to write the entire code inside a single function. But when doing so we have to use exec() instead of open()
wmayer wrote: Sat Oct 22, 2022 4:37 pm git commit c5a7654285
That's very elegant solution. Kudos.
Glad I can close this tab. :)
Post Reply