diff options
author | VaL Doroshchuk <valentyn.doroshchuk@qt.io> | 2020-02-04 13:18:31 +0100 |
---|---|---|
committer | VaL Doroshchuk <valentyn.doroshchuk@qt.io> | 2020-02-05 11:05:24 +0100 |
commit | 9ecc595d7185ad3d072f3d811d7aa52fc6992cca (patch) | |
tree | 5bb1dc10f0b57f41ac76385bbe8f2b21a567ef83 | |
parent | e5408b62bdcee1abbc595eb581abcb540396ca4c (diff) |
widgets: Don't create winId when the widget is being destroyed
When QWidget is being destroyed, its winId is cleared, and
a QEvent::WinIdChange is sent. If a listener of this event
reacted by calling winId() again, we might crash.
A crash can be observed when this child widget is destroyed in dtor of its parent.
E.g. here is a hierarchy of widgets:
1:QWidget
2:QObject
3:QWidget
4:QWidget
If a listener subscribed for WinIdChange events from (4),
and there is a connection to destroy (4) when (2) is destroyed.
This will lead to infinite loop:
1. QWidget::~QWidget
2. QWidget::destroy
3. QWidgetPrivate::setWinId(0)
4. QCoreApplication::sendEvent(q, QEvent::WinIdChange);
5. eventFilter
6. QWidget::winId
7. QWidgetPrivate::createWinId (this=0x555555957600) at kernel/qwidget.cpp:2380
8. QWidgetPrivate::createWinId (this=0x55555596b040) at kernel/qwidget.cpp:2387
9. QWidget::create (this=0x5555558f2010, window=0, initializeWindow=true, destroyOldWindow=true) at kernel/qwidget.cpp:1163
10. QWidgetPrivate::createWinId (this=0x55555596b040) at kernel/qwidget.cpp:2387
11. QWidget::create (this=0x5555558f2010, window=0, initializeWindow=true, destroyOldWindow=true) at kernel/qwidget.cpp:1163
12. QWidgetPrivate::createWinId (this=0x55555596b040) at kernel/qwidget.cpp:2387
Fixes: QTBUG-81849
Change-Id: Ib4c33ac97d9a79c701431ae107bddfb22720ba0d
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
-rw-r--r-- | src/widgets/kernel/qwidget.cpp | 4 | ||||
-rw-r--r-- | tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp | 51 |
2 files changed, 54 insertions, 1 deletions
diff --git a/src/widgets/kernel/qwidget.cpp b/src/widgets/kernel/qwidget.cpp index 8ec7f7e072..e7f95ecf4d 100644 --- a/src/widgets/kernel/qwidget.cpp +++ b/src/widgets/kernel/qwidget.cpp @@ -2350,7 +2350,9 @@ QWidget *QWidget::find(WId id) */ WId QWidget::winId() const { - if (!testAttribute(Qt::WA_WState_Created) || !internalWinId()) { + if (!data->in_destructor + && (!testAttribute(Qt::WA_WState_Created) || !internalWinId())) + { #ifdef ALIEN_DEBUG qDebug() << "QWidget::winId: creating native window for" << this; #endif diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index b5d174f340..2dd9e13b80 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -409,6 +409,8 @@ private slots: void closeEvent(); void closeWithChildWindow(); + void winIdAfterClose(); + private: bool ensureScreenSize(int width, int height); @@ -11261,5 +11263,54 @@ void tst_QWidget::closeWithChildWindow() QVERIFY(!childWidget->isVisible()); } +class WinIdChangeSpy : public QObject +{ + Q_OBJECT +public: + QWidget *widget = nullptr; + WId winId = 0; + explicit WinIdChangeSpy(QWidget *w, QObject *parent = nullptr) + : QObject(parent) + , widget(w) + , winId(widget->winId()) + { + } + +public slots: + bool eventFilter(QObject *obj, QEvent *event) override + { + if (obj == widget) { + if (event->type() == QEvent::WinIdChange) { + winId = widget->winId(); + return true; + } + } + return false; + } +}; + +void tst_QWidget::winIdAfterClose() +{ + auto widget = new QWidget; + auto notifier = new QObject(widget); + auto deleteWidget = new QWidget(new QWidget(widget)); + auto spy = new WinIdChangeSpy(deleteWidget); + deleteWidget->installEventFilter(spy); + connect(notifier, &QObject::destroyed, [&] { delete deleteWidget; }); + + widget->setAttribute(Qt::WA_NativeWindow); + widget->windowHandle()->create(); + widget->show(); + + QVERIFY(QTest::qWaitForWindowExposed(widget)); + QVERIFY(spy->winId); + + widget->windowHandle()->close(); + delete widget; + + QCOMPARE(spy->winId, WId(0)); + delete spy; +} + QTEST_MAIN(tst_QWidget) #include "tst_qwidget.moc" |