Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when exiting #730

Open
jaskonius opened this issue Jan 5, 2025 · 9 comments
Open

Crash when exiting #730

jaskonius opened this issue Jan 5, 2025 · 9 comments

Comments

@jaskonius
Copy link

Note: this might reference #715.

I've built Notes from source a few minutes ago to give it a run and stumbled upon both a bug and a crash that might be related to it. When I open the settings they are not rendered properly as can be seen below. Furthermore, the window cannot be moved since it closes immediately when clicking anywhere but in content area. Clicking in the area where my mouse is apparently counts as 'in content area' since the window does not close.
image

So much on the bug.

The crash occours every time when closing the application after the settings window has been used (this might relate to #715).
I have no experience with gdb, all I can provide for now is

Thread 1 "notes" received signal SIGSEGV, Segmentation fault.
0x00007ffff5746804 in QWindow::parent(QWindow::AncestorMode) const ()
    from /lib64/libQt6Gui.so.6

Build output (stripped):

$ cmake .. -DCMAKE_BUILD_TYPE=Debug
-- The CXX compiler identification is GNU 14.2.1
-- Performing Test HAVE_STDATOMIC - Success
-- Found WrapAtomic: TRUE
-- Found OpenGL: /usr/lib64/libOpenGL.so
-- Found WrapOpenGL: TRUE
-- Found XKB: /usr/lib64/libxkbcommon.so (found suitable version "1.7.0", minimum required is "0.5.0")
-- Found WrapVulkanHeaders: /usr/include
-- Success! Configuration details:
-- App name: Notes
-- App version: 2.3.1
-- Qt version: 6.8.1
-- Update checker: ON
-- Pro Version: ON
-- Build type: Debug
CMake Warning (dev) at /usr/lib64/cmake/Qt6Qml/Qt6QmlMacros.cmake:3051 (message):
  src/qml/Utilities.js is not an ECMAScript module and also doesn't contain
  '.pragma library'.  It will be re-evaluated in the context of every QML
  document that explicitly or implicitly imports io.github.nuttyartist.notes.
  Set its QT_QML_SKIP_QMLDIR_ENTRY source file property to FALSE if you
  really want this to happen.  Set it to TRUE to prevent it.
Call Stack (most recent call first):
  /usr/lib64/cmake/Qt6Qml/Qt6QmlMacros.cmake:896 (qt6_target_qml_sources)
  /usr/lib64/cmake/Qt6Qml/Qt6QmlMacros.cmake:1232 (qt6_add_qml_module)
  CMakeLists.txt:328 (qt_add_qml_module)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Found X11: /usr/include
-- Looking for XOpenDisplay in /usr/lib64/libX11.so;/usr/lib64/libXext.so
-- Looking for XOpenDisplay in /usr/lib64/libX11.so;/usr/lib64/libXext.so - found
-- Looking for gethostbyname - found
-- Looking for connect - found
-- Looking for remove - found
-- Looking for shmat - found

Make yields the following warning (stripped absolute path to notes folder):

notes/src/mainwindow.cpp: In member function ‘virtual bool MainWindow::eventFilter(QObject*, QEvent*)’:
notes/src/mainwindow.cpp:3921:42: warning: ‘static void QApplication::setActiveWindow(QWidget*)’ is deprecated: Use QWidget::activateWindow() instead. [-Wdeprecated-declarations]
 3921 |             QApplication::setActiveWindow(
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
 3922 |                     this); // TODO: The docs say this function is deprecated but it's the only one
      |                     ~~~~~                 
In file included from /usr/include/qt6/QtWidgets/QApplication:1,
                 from notes/build/Notes_autogen/include/ui_mainwindow.h:13,
                 from notes/src/mainwindow.cpp:8:
/usr/include/qt6/QtWidgets/qapplication.h:84:17: note: declared here
   84 |     static void setActiveWindow(QWidget* act);
      |    

I run Fedora 41 with GNOME DE (which should provide wayland? cmake found x11 however).

@nuttyartist
Copy link
Owner

Hi there!

I've built Notes from source a few minutes ago to give it a run and stumbled upon both a bug and a crash that might be related to it. When I open the settings they are not rendered properly as can be seen below. Furthermore, the window cannot be moved since it closes immediately when clicking anywhere but in content area. Clicking in the area where my mouse is apparently counts as 'in content area' since the window does not close.

What you describe here is not really a bug. Since the EditorSettings pane is written in QML and the rest of the app (except the Kanban view) is Qt Widgets, I had to create a new window that will behave like a Popup which leads to some inconsistent behavior you're experiencing. In my new note-taking app (https://get-notes.com/) the editor is written in QML so that problem is gone since the EditorSettings can be part of the QML scene all the time.

The ideal solution to this problem would be to convert the editor to QML.

Regarding the crash. Can you show a video how to reproduce the crash? At least here on macOS I can't reproduce it.

@jaskonius
Copy link
Author

Thanks for the quick reply!

The screencast is attached below.
Since I am apparently the only one with those problems, it is probably just something with my system.
Screencast From 2025-01-06 11-15-27.webm

@guihkx
Copy link
Collaborator

guihkx commented Jan 6, 2025

Thanks for the video.

I can also reproduce the crash here by just opening Notes' settings window, closing it, and then quitting Notes.

  • OS: Arch Linux
  • Qt: 6.8.1
  • Desktop environment: Plasma 6.2.5
  • Display server: X11

Build options:

-DUPDATE_CHECKER=ON -DCMAKE_BUILD_TYPE=Debug

@guihkx
Copy link
Collaborator

guihkx commented Jan 6, 2025

The crash does not reproduce with the v2.3.1 AppImage (which still uses Qt 6.4.3).

Just to rule this out as a problem on our side, I ran git checkout v2.3.1, built that with Qt 6.8.1, but it still crashes.

Maybe this points to a Qt regression?

Unfortunately, the gdb backtrace is obscure to me (this is on 57542f6 btw):

❯ gdb -ex run --args ./build/notes
Reading symbols from ./build/notes...
Starting program: ./build/notes 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
[New Thread 0x7ffff11ff6c0 (LWP 19369)]
[New Thread 0x7ffff09fe6c0 (LWP 19370)]
[New Thread 0x7fffe886b6c0 (LWP 19378)]
[New Thread 0x7fffe13bb6c0 (LWP 19379)]
[New Thread 0x7fffdbfff6c0 (LWP 19380)]
[New Thread 0x7fffdadba6c0 (LWP 19381)]
[New Thread 0x7fffd99be6c0 (LWP 19390)]
[New Thread 0x7fffbf3fe6c0 (LWP 19392)]
[New Thread 0x7fffbebfd6c0 (LWP 19393)]
[New Thread 0x7fffbe3fc6c0 (LWP 19394)]
[New Thread 0x7fffbdbfb6c0 (LWP 19395)]
[New Thread 0x7fffa7fff6c0 (LWP 19396)]
[New Thread 0x7fffa77fe6c0 (LWP 19397)]
[New Thread 0x7fffa6ffd6c0 (LWP 19398)]
[New Thread 0x7fffa67fc6c0 (LWP 19399)]
[New Thread 0x7fffa5ffb6c0 (LWP 19400)]
[New Thread 0x7fffa57fa6c0 (LWP 19403)]
[New Thread 0x7fffa4ff96c0 (LWP 19404)]
[Thread 0x7fffd99be6c0 (LWP 19390) exited]
[Thread 0x7fffdadba6c0 (LWP 19381) exited]
[Thread 0x7fffdbfff6c0 (LWP 19380) exited]
[Thread 0x7fffa57fa6c0 (LWP 19403) exited]
[Thread 0x7fffe13bb6c0 (LWP 19379) exited]
[Thread 0x7fffe886b6c0 (LWP 19378) exited]

Thread 1 "notes" received signal SIGSEGV, Segmentation fault.
QWindow::parent (this=0x0, mode=mode@entry=QWindow::ExcludeTransients) at /usr/src/debug/qt6-base/qtbase/src/gui/kernel/qwindow.cpp:771
771         Q_D(const QWindow);
(gdb) bt
#0  QWindow::parent (this=0x0, mode=mode@entry=QWindow::ExcludeTransients) at /usr/src/debug/qt6-base/qtbase/src/gui/kernel/qwindow.cpp:771
#1  0x00007ffff6f67330 in QWindowContainer::toplevelAboutToBeDestroyed (parent=0x5555570632d0) at /usr/src/debug/qt6-base/qtbase/src/widgets/kernel/qwindowcontainer.cpp:391
#2  0x00007ffff6f37071 in QWidgetPrivate::deleteTLSysExtra (this=0x5555578b19e0) at /usr/src/debug/qt6-base/qtbase/src/widgets/kernel/qwidget.cpp:1709
#3  0x00007ffff6f548a8 in QWidget::destroy (this=0x5555570632d0, destroyWindow=<optimized out>, destroySubWindows=<optimized out>)
    at /usr/src/debug/qt6-base/qtbase/src/widgets/kernel/qwidget.cpp:12571
#4  0x00007ffff6ef5b3f in QApplication::~QApplication (this=<optimized out>, this=<optimized out>) at /usr/src/debug/qt6-base/qtbase/src/widgets/kernel/qapplication.cpp:686
#5  0x000055555564dbab in main (argc=1, argv=0x7fffffffdde8) at /home/gui/dev/notes/src/main.cpp:104
(gdb)

@guihkx
Copy link
Collaborator

guihkx commented Jan 6, 2025

So I found out that this regression(?) was introduced with Qt 6.7.3, and after bisecting qtbase between 6.7.2 (good) and 6.7.3 (bad), I found the culprit commit:

commit 0673dde3f526db9cd62657a60783006d4cee0db2 (HEAD)
Author: Tor Arne Vestbø <[email protected]>
Date:   Mon Jun 17 19:34:28 2024 +0200

    Let QWindowContainer know when its top level is about to be destroyed
    
    When the top level window that a QWindowContainer is in is about to
    be destroyed the QWindowContainer must reparent the contained window
    into a dummy window, as otherwise the destruction of the top level
    will bring down the contained window as well.
    
    We were notifying the window container about this situation when
    the window container was moved from being a top level itself, to
    being a child widget, but did not have any logic for other ways
    the window container could lose its parent QWindow.
    
    An example of this was when RHI-needs would result in recreating
    the top revel with a different RHI backend.
    
    We now have a last minute call to toplevelAboutToBeDestroyed in
    QWidgetPrivate::deleteTLSysExtra().
    
    Fixes: QTBUG-126303
    Pick-to: 6.5
    Change-Id: I5b14e156956ae76a8f53cac31904eaadee9e791f
    Reviewed-by: Volker Hilsheimer <[email protected]>
    (cherry picked from commit 006cbf658ea1f5986bbe1baafa7c146780320661)
    Reviewed-by: Qt Cherry-pick Bot <[email protected]>
    (cherry picked from commit f0f9cc602f065e167a0ac608203d7a5fc3a3c297)
    Reviewed-by: Tor Arne Vestbø <[email protected]>

 src/widgets/kernel/qwidget.cpp                                      |  4 ++++
 tests/auto/widgets/kernel/qwindowcontainer/tst_qwindowcontainer.cpp | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

This tiny change prevents the crash on my machine (please test it if you can and report back):

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 186cdbb..496c01d 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1047,7 +1047,7 @@ void MainWindow::setupEditorSettings()
     m_editorSettingsQuickView.setResizeMode(QQuickView::SizeViewToRootObject);
     m_editorSettingsQuickView.setFlags(Qt::FramelessWindowHint);
     m_editorSettingsQuickView.setColor(Qt::transparent);
-    m_editorSettingsWidget = QWidget::createWindowContainer(&m_editorSettingsQuickView, nullptr);
+    m_editorSettingsWidget = QWidget::createWindowContainer(&m_editorSettingsQuickView, this);
 #if defined(Q_OS_MACOS)
 #  if QT_VERSION >= QT_VERSION_CHECK(6, 2, 0)
     m_editorSettingsWidget->setWindowFlags(Qt::Popup | Qt::FramelessWindowHint

Though I'm not entirely sure if that's the correct fix.

@nuttyartist: Would specifying the main window as the parent of the editor settings widget mess up something on macOS/Windows (visually)? Because on Linux there's no difference, as far as I can tell.

@jaskonius
Copy link
Author

Can confirm that prevents the crash for me.

@nuttyartist
Copy link
Owner

Whoops.

@guihkx thanks for the detective work! I forgot why I didn't assign it a parent. I tested now on macOS, it seems to work well. I'll test on Windows and if that works I'll send a PR.

@guihkx
Copy link
Collaborator

guihkx commented Jan 7, 2025

To be fair, it didn't crash at all before 6.7.3, so it's hardly your fault that it does now...

Though there are a couple of other places where you also call QWidget::createWindowContainer without a parent, and I'm not sure if they can also trigger a crash elsewhere, so you might want to look into those too...

@zjeffer
Copy link
Collaborator

zjeffer commented Jan 20, 2025

I believe we always have to set the parent to something, instead of setting it to nullptr. If we don't, the deleter is never called and we either have memory leaks or apparently also crashes like this one.

I fixed some of them in this PR: #731

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants