Skip to content

Commit

Permalink
Fix crash with the software renderer and windows with QObject parent
Browse files Browse the repository at this point in the history
When a QQuickWindow is a child of another QObject (such as a Loader) and
is scheduled for deletion using a deferred delete event, then a deletion
via the parent ends up calling the window's destructor, which will
finally end up in ~QObject(), which takes care of removing the posted
deferred deletion event from the event queue.

In the case of QQuickWindow, the destructor - called before ~QObject -
calls windowDestroyed(this) on the SG render loop. The implementation in
the software renderer calls QCoreApplication::sendPostedEvents() with
QEvent::DeferedDelete, which ends up deleting the same window a second
time and resulting in a crash.

I can't see a good reason for the existence of the sendPostedEvents()
call there. It is not present in the other render loops and according to
git blame it stems from the very early first implementation of the
software renderer where surely copy & paste from other render loop code
was involved back then.

The same fix is applied to the single-threaded VG and D3D12 render
loops, as they are most likely copy & paste from the software render
loop implementation.

ASAN trace for tst_qquickwindow::unloadSubWindow() on 5.11 branch that shows
invalid access to the QObjectPrivate/QQuickWindowPrivate, which follows the
QObject in terms of life-cycle:

==4736==ERROR: AddressSanitizer: heap-use-after-free on address 0x617000011778 at pc 0x7fdd211cfbb3 bp 0x7fffecb47ea0 sp 0x7fffecb47e90
READ of size 8 at 0x617000011778 thread T0
    #0 0x7fdd211cfbb2 in QQuickWindow::~QQuickWindow() items/qquickwindow.cpp:1308
    #1 0x7fdd21470974 in QQuickWindowQmlImpl::~QQuickWindowQmlImpl() items/qquickwindowmodule_p.h:63
    #2 0x7fdd21470974 in QQmlPrivate::QQmlElement<QQuickWindowQmlImpl>::~QQmlElement() .../qqmlprivate.h:103
    #3 0x7fdd21470974 in QQmlPrivate::QQmlElement<QQuickWindowQmlImpl>::~QQmlElement() .../qqmlprivate.h:103
    #4 0x7fdd1e24da24 in qDeleteInEventHandler(QObject*) kernel/qobject.cpp:4601
    #5 0x7fdd1e253d2f in QObject::event(QEvent*) kernel/qobject.cpp:1240
    #6 0x7fdd1fbd1d41 in QWindow::event(QEvent*) kernel/qwindow.cpp:2356
    #7 0x7fdd211f778e in QQuickWindow::event(QEvent*) items/qquickwindow.cpp:1628
    #8 0x7fdd1e1a4e3c in QCoreApplicationPrivate::notify_helper(QObject*, QEvent*) kernel/qcoreapplication.cpp:1216
    #9 0x7fdd1e1a508b in doNotify kernel/qcoreapplication.cpp:1157
    #10 0x7fdd1e1a555a in QCoreApplication::notify(QObject*, QEvent*) kernel/qcoreapplication.cpp:1143
    #11 0x7fdd1fb87665 in QGuiApplication::notify(QObject*, QEvent*) kernel/qguiapplication.cpp:1723
    #12 0x7fdd1e1a52fa in QCoreApplication::notifyInternal2(QObject*, QEvent*) kernel/qcoreapplication.cpp:1067
    #13 0x7fdd1e1b6ed2 in QCoreApplication::sendEvent(QObject*, QEvent*) ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:234
    #14 0x7fdd1e1b6ed2 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) kernel/qcoreapplication.cpp:1764
    #15 0x7fdd1e1b8cda in QCoreApplication::sendPostedEvents(QObject*, int) kernel/qcoreapplication.cpp:1618
    #16 0x7fdd210cb034 in QSGSoftwareRenderLoop::windowDestroyed(QQuickWindow*) scenegraph/adaptations/software/qsgsoftwarerenderloop.cpp:100
    #17 0x7fdd211cfb8c in QQuickWindow::~QQuickWindow() items/qquickwindow.cpp:1305
[...]

0x617000011778 is located 632 bytes inside of 704-byte region [0x617000011500,0x6170000117c0)
freed by thread T0 here:
    #0 0x7fdd21a8a9d8 in operator delete(void*, unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xe19d8)
    #1 0x7fdd2146fa3c in QQuickWindowQmlImplPrivate::~QQuickWindowQmlImplPrivate() items/qquickwindowmodule.cpp:57
    #2 0x7fdd1e26b252 in QScopedPointerDeleter<QObjectData>::cleanup(QObjectData*) [...]
    #3 0x7fdd1e26b252 in QScopedPointer<QObjectData, QScopedPointerDeleter<QObjectData> >::~QScopedPointer() [...]
    #4 0x7fdd1e26b252 in QObject::~QObject() kernel/qobject.cpp:882
    #5 0x7fdd1fbcf51c in QWindow::~QWindow() kernel/qwindow.cpp:211
    #6 0x7fdd211d0466 in QQuickWindow::~QQuickWindow() items/qquickwindow.cpp:1297
    #7 0x7fdd211d0644 in QQuickWindow::~QQuickWindow() items/qquickwindow.cpp:1335
    #8 0x7fdd1e2666b4 in QObjectPrivate::deleteChildren() kernel/qobject.cpp:1995
    #9 0x7fdd1e26b329 in QObject::~QObject() kernel/qobject.cpp:1023
[...]

Change-Id: Iffa90d365d02b074e2a78c5be2895c9f86a4b80e
Task-number: QTBUG-66381
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
(cherry picked from commit 238cc09)
  • Loading branch information
tronical committed Feb 15, 2018
1 parent b242078 commit 557e762
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 5 deletions.
3 changes: 0 additions & 3 deletions src/plugins/scenegraph/d3d12/qsgd3d12renderloop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,6 @@ void QSGD3D12RenderLoop::windowDestroyed(QQuickWindow *window)

rc->invalidate();

if (m_windows.isEmpty())
QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);

delete rc;
delete engine;

Expand Down
1 change: 0 additions & 1 deletion src/plugins/scenegraph/openvg/qsgopenvgrenderloop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ void QSGOpenVGRenderLoop::windowDestroyed(QQuickWindow *window)
rc->invalidate();
delete vg;
vg = nullptr;
QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
} else if (vg && window == vg->window()) {
vg->doneCurrent();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ void QSGSoftwareRenderLoop::windowDestroyed(QQuickWindow *window)

if (m_windows.size() == 0) {
rc->invalidate();
QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
}
}

Expand Down

0 comments on commit 557e762

Please sign in to comment.