wangwenx190 / framelesshelper

Project moved to: https://github.com/stdware/qwindowkit Cross-platform window customization framework for Qt Widgets and Qt Quick. Supports Windows, Linux and macOS.
MIT License
849 stars 202 forks source link

Windows deleted before shown crash #304

Open ollie-dawes opened 1 year ago

ollie-dawes commented 1 year ago

As of commit: c4a7bc80d0ca0b1b87418515ab4836f3d4944ae0

Application can crash on Windows (Qt 6.6) in the FramelessHelperHookWindowProc callback if the underlying QWindow has already been deleted.

Ran into this issue when testing out the latest https://github.com/KDAB/KDDockWidgets, the below crash happens after the window is "docked".

1  QQuickItem::window                                                                                                                   qquickitem.cpp                 2870 0x7ff96155e11e 
2  `wangwenx190::FramelessHelper::FramelessQuickHelperPrivate::attach'::`11'::<lambda_13>::operator()                                   framelessquickhelper.cpp       201  0x7ff7e15a6564 
3  std::invoke<`wangwenx190::FramelessHelper::FramelessQuickHelperPrivate::attach'::`11'::<lambda_13> &>                                type_traits                    1586 0x7ff7e15b72e0 
4  std::_Invoker_ret<QWindow *,0>::_Call<`wangwenx190::FramelessHelper::FramelessQuickHelperPrivate::attach'::`11'::<lambda_13> &>      functional                     754  0x7ff7e15b0f60 
5  std::_Func_impl_no_alloc<`wangwenx190::FramelessHelper::FramelessQuickHelperPrivate::attach'::`11'::<lambda_13>,QWindow *>::_Do_call functional                     921  0x7ff7e15a8987 
6  std::_Func_class<QWindow *>::operator()                                                                                              functional                     969  0x7ff7e16219ab 
7  wangwenx190::FramelessHelper::FramelessHelperHookWindowProc                                                                          utils_win.cpp                  925  0x7ff7e1615e86 
8  CallWindowProcW                                                                                                                      USER32                              0x7ffa0e67e858 
9  DispatchMessageW                                                                                                                     USER32                              0x7ffa0e67e3dc 
10 SendMessageTimeoutW                                                                                                                  USER32                              0x7ffa0e690c93 
11 KiUserCallbackDispatcher                                                                                                             ntdll                               0x7ffa0ec10e64 
12 NtUserShowWindow                                                                                                                     win32u                              0x7ffa0c741b24 
13 QWindowsWindow::setVisible                                                                                                           qwindowswindow.cpp             1744 0x7ff95baed54d 
14 QWindowPrivate::setVisible                                                                                                           qwindow.cpp                    411  0x7ff95eaade83 
15 QWindow::setVisible                                                                                                                  qwindow.cpp                    673  0x7ff95eaaa07c 
16 QWindowPrivate::destroy                                                                                                              qwindow.cpp                    2042 0x7ff95eaae90c 
17 QWindow::~QWindow                                                                                                                    qwindow.cpp                    186  0x7ff95eaa68c2 
18 QQuickWindow::~QQuickWindow                                                                                                          qquickwindow.cpp               1134 0x7ff961780913 
19 QQuickView::~QQuickView                                                                                                              qquickview.cpp                 176  0x7ff961770511 
20 KDDockWidgets::QuickView::~QuickView                                                                                                 FloatingWindow.cpp             130  0x7ff7e155cf84 
21 KDDockWidgets::QuickView::`scalar deleting destructor'                                                                               canonic_exe                         0x7ff7e155ea98 
22 KDDockWidgets::QtQuick::FloatingWindow::~FloatingWindow                                                                              FloatingWindow.cpp             151  0x7ff7e155c1fa 
23 KDDockWidgets::QtQuick::FloatingWindow::`scalar deleting destructor'                                                                 canonic_exe                         0x7ff7e155ea48 
24 KDDockWidgets::Core::View::Private::free                                                                                             View.cpp                       100  0x7ff7e133aae2 
25 KDDockWidgets::Core::Controller::~Controller                                                                                         Controller.cpp                 37   0x7ff7e1334f36 
26 KDDockWidgets::Core::FloatingWindow::~FloatingWindow                                                                                 FloatingWindow.cpp             281  0x7ff7e147e880 
27 KDDockWidgets::Core::FloatingWindow::`scalar deleting destructor'                                                                    canonic_exe                         0x7ff7e1487e88 
28 KDDockWidgets::Core::DelayedDelete::call                                                                                             DelayedCall.cpp                41   0x7ff7e1469d85 
29 <lambda_5ecccb2e996ef077d41c7c2a03f7ad8e>::operator()                                                                                Platform.cpp                   321  0x7ff7e1531080 
30 QtPrivate::FunctorCall<QtPrivate::IndexesList<>,QtPrivate::List<>,void,<lambda_5ecccb2e996ef077d41c7c2a03f7ad8e>>::call              qobjectdefs_impl.h             137  0x7ff7e15315b9 
31 QtPrivate::Functor<<lambda_5ecccb2e996ef077d41c7c2a03f7ad8e>,0>::call<QtPrivate::List<>,void>                                        qobjectdefs_impl.h             340  0x7ff7e15327f3 
32 QtPrivate::QCallableObject<<lambda_5ecccb2e996ef077d41c7c2a03f7ad8e>,QtPrivate::List<>,void>::impl                                   qobjectdefs_impl.h             534  0x7ff7e15313d0 
33 QtPrivate::QSlotObjectBase::call                                                                                                     qobjectdefs_impl.h             433  0x7ff95c8aed53 
34 QMetaCallEvent::placeMetaCall                                                                                                        qobject.cpp                    649  0x7ff95c966e6f 
35 QObject::event                                                                                                                       qobject.cpp                    1437 0x7ff95c95da4d 
36 QCoreApplication::event                                                                                                              qcoreapplication.cpp           2030 0x7ff95c898df8 
37 QGuiApplication::event                                                                                                               qguiapplication.cpp            2025 0x7ff95e992337 
38 QCoreApplicationPrivate::notify_helper                                                                                               qcoreapplication.cpp           1285 0x7ff95c89aed2 
39 doNotify                                                                                                                             qcoreapplication.cpp           1214 0x7ff95c89d753 
40 QCoreApplication::notify                                                                                                             qcoreapplication.cpp           1198 0x7ff95c8971e8 
41 QGuiApplication::notify                                                                                                              qguiapplication.cpp            1990 0x7ff95e9912af 
42 QCoreApplication::notifyInternal2                                                                                                    qcoreapplication.cpp           1118 0x7ff95c8994a7 
43 QCoreApplication::sendEvent                                                                                                          qcoreapplication.cpp           1537 0x7ff95c89685b 
44 QCoreApplicationPrivate::sendPostedEvents                                                                                            qcoreapplication.cpp           1898 0x7ff95c89bb52 
45 QEventDispatcherWin32::sendPostedEvents                                                                                              qeventdispatcher_win.cpp       901  0x7ff95cce339f 
46 QWindowsGuiEventDispatcher::sendPostedEvents                                                                                         qwindowsguieventdispatcher.cpp 44   0x7ff95f139a45 
47 QEventDispatcherWin32::processEvents                                                                                                 qeventdispatcher_win.cpp       471  0x7ff95cce169f 
48 QWindowsGuiEventDispatcher::processEvents                                                                                            qwindowsguieventdispatcher.cpp 36   0x7ff95f1399fb 
49 QEventLoop::processEvents                                                                                                            qeventloop.cpp                 101  0x7ff95c8bf82c 
50 QEventLoop::exec                                                                                                                     qeventloop.cpp                 182  0x7ff95c8bfb14 
51 QCoreApplication::exec                                                                                                               qcoreapplication.cpp           1439 0x7ff95c8965fa 
52 QGuiApplication::exec                                                                                                                qguiapplication.cpp            1922 0x7ff95e99123a 
53 main                                                                                                                                 main.cpp                       123  0x7ff7e1254869 
54 invoke_main                                                                                                                          exe_common.inl                 79   0x7ff7e1650c89 
55 __scrt_common_main_seh                                                                                                               exe_common.inl                 288  0x7ff7e1650b6e 
56 __scrt_common_main                                                                                                                   exe_common.inl                 331  0x7ff7e1650a2e 
57 mainCRTStartup                                                                                                                       exe_main.cpp                   17   0x7ff7e1650d1e 
58 BaseThreadInitThunk                                                                                                                  KERNEL32                            0x7ffa0cde7344 
59 RtlUserThreadStart                                                                                                                   ntdll                               0x7ffa0ebc26b1 

The above crash is due to the following capture of the FramelessQuickHelper* in a lambda here: image

Which is invoked by the FramelessHelperHookWindowProc callback here (line 912): image

uMsg in this case is WM_SHOWWINDOW for what it is worth.

The following patch fixes the issue on my end by capturing a QPointer to the QWindow instead of the raw FramelessQuickHelper*:

diff --git a/src/quick/framelessquickhelper.cpp b/src/quick/framelessquickhelper.cpp
index e4cf14e..3f53d8d 100644
--- a/src/quick/framelessquickhelper.cpp
+++ b/src/quick/framelessquickhelper.cpp
@@ -184,6 +184,8 @@ void FramelessQuickHelperPrivate::attach()
         return;
     }

+    QPointer<QWindow> w = window;
+
     if (!data->callbacks) {
         data->callbacks = FramelessCallbacks::create();
         data->callbacks->getWindowId = [window]() -> WId { return window->winId(); };
@@ -198,7 +200,7 @@ void FramelessQuickHelperPrivate::attach()
         data->callbacks->setWindowFixedSize = [q](const bool value) -> void { q->setWindowFixedSize(value); };
         data->callbacks->getWindowState = [window]() -> Qt::WindowState { return window->windowState(); };
         data->callbacks->setWindowState = [window](const Qt::WindowState state) -> void { window->setWindowState(state); };
-        data->callbacks->getWindowHandle = [q]() -> QWindow * { return q->window(); };
+        data->callbacks->getWindowHandle = [w]() -> QWindow * { return w; };
         data->callbacks->windowToScreen = [window](const QPoint &pos) -> QPoint { return window->mapToGlobal(pos); };
         data->callbacks->screenToWindow = [window](const QPoint &pos) -> QPoint { return window->mapFromGlobal(pos); };
         data->callbacks->isInsideSystemButtons = [this](const QPoint &pos, SystemButtonType *button) -> bool {

fix.patch

wangwenx190 commented 1 year ago

Really thanks for your excellent solution! Using QPointer is really a good idea. But can you make a PR for it?

wangwenx190 commented 1 year ago

But this issue also makes me worried. Currently FramelessHelper is using QWindow pointer to identify different instances, I didn't account for the case that the QWindow itself is destroyed (because currently I don't have a good way to get notified before a QWindow is about to be destroyed, the QObject::destroyed signal is too late), it may be a potential bug source.

ollie-dawes commented 1 year ago

Really thanks for your excellent solution! Using QPointer is really a good idea. But can you make a PR for it?

Sure, here's a pr with the fix: https://github.com/wangwenx190/framelesshelper/pull/305

I would not be surprised if there are other lifetime issues as well with some of these other lambda callbacks but so far I have not seen any other issues.

wangwenx190 commented 1 year ago

Sure, here's a pr with the fix: https://github.com/wangwenx190/framelesshelper/pull/305

Thanks! It has been merged just now.

I would not be surprised if there are other lifetime issues as well with some of these other lambda callbacks but so far I have not seen any other issues.

It seems this lambda dispatching mechanism is a little more fragile than I would expect, but this is the first crash report so far, so maybe it's not that fragile 😂