From 9b4fbe85d2e00c625c3d4abd975faf555000f685 Mon Sep 17 00:00:00 2001 From: Giulio Camuffo Date: Sun, 31 Aug 2014 16:16:53 +0300 Subject: Add a function for QPA plugins to explicitly destroy QScreens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously QPlatformScreen was automatically deleting its QScreen in ~QPlatformScreen(). That means that we cannot use QScreen's methods when the screen is being removed, because doing so would call virtual methods of QPlatformScreen. By that point the QPlatformScreen subclass object does not exist anymore, and we call the default implementation instead of the subclassed one, or get a crash for the pure virtual methods. This happens for example when removing a screen which contains a QWindow with some QML item using QQuickScreenAttached. This patch adds a QPlatformIntegration::destroyScreen() function, which deletes the QScreen and later the QPlatformScreen. ~QPlatformScreen will still delete the QScreen if it was not deleted with destroyScreen(), so code not ported to the new approach will continue to work as before, with only a warning added. Task-number: QTBUG-41141 Change-Id: Ie4a03dee08ceb4c3e94a81875411f6f723273fe1 Reviewed-by: Jørgen Lind Reviewed-by: Shawn Rutledge --- src/gui/kernel/qplatformintegration.cpp | 18 +++++++++++++++++- src/gui/kernel/qplatformintegration.h | 1 + src/gui/kernel/qplatformscreen.cpp | 8 +++++--- src/plugins/platforms/cocoa/qcocoaintegration.mm | 4 ++-- src/plugins/platforms/ios/qiosintegration.mm | 2 +- src/plugins/platforms/kms/qkmsintegration.cpp | 2 +- src/plugins/platforms/linuxfb/qlinuxfbintegration.cpp | 2 +- .../platforms/minimalegl/qminimaleglintegration.cpp | 2 +- src/plugins/platforms/openwfd/qopenwfdintegration.cpp | 5 +++++ src/plugins/platforms/openwfd/qopenwfdintegration.h | 1 + src/plugins/platforms/openwfd/qopenwfdport.cpp | 2 +- src/plugins/platforms/qnx/qqnxintegration.cpp | 2 +- src/plugins/platforms/windows/qwindowsintegration.h | 1 + src/plugins/platforms/windows/qwindowsscreen.cpp | 9 ++++++++- src/plugins/platforms/windows/qwindowsscreen.h | 6 +----- src/plugins/platforms/xcb/qxcbconnection.cpp | 8 +++++--- 16 files changed, 52 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/gui/kernel/qplatformintegration.cpp b/src/gui/kernel/qplatformintegration.cpp index 7e291e9050..86edb9bd28 100644 --- a/src/gui/kernel/qplatformintegration.cpp +++ b/src/gui/kernel/qplatformintegration.cpp @@ -439,7 +439,7 @@ QList QPlatformIntegration::possibleKeys(const QKeyEvent *) const This adds the screen to QGuiApplication::screens(), and emits the QGuiApplication::screenAdded() signal. - The screen is automatically removed when the QPlatformScreen is destroyed. + The screen should be deleted by calling QPlatformIntegration::destroyScreen(). */ void QPlatformIntegration::screenAdded(QPlatformScreen *ps) { @@ -449,6 +449,22 @@ void QPlatformIntegration::screenAdded(QPlatformScreen *ps) emit qGuiApp->screenAdded(screen); } +/*! + Should be called by the implementation whenever a screen is removed. + + This removes the screen from QGuiApplication::screens(), and deletes it. + + Failing to call this and manually deleting the QPlatformScreen instead may + lead to a crash due to a pure virtual call. +*/ +void QPlatformIntegration::destroyScreen(QPlatformScreen *screen) +{ + QGuiApplicationPrivate::screen_list.removeOne(screen->d_func()->screen); + delete screen->d_func()->screen; + screen->d_func()->screen = Q_NULLPTR; + delete screen; +} + QStringList QPlatformIntegration::themeNames() const { return QStringList(); diff --git a/src/gui/kernel/qplatformintegration.h b/src/gui/kernel/qplatformintegration.h index ccbe4cc73d..dbef939b8b 100644 --- a/src/gui/kernel/qplatformintegration.h +++ b/src/gui/kernel/qplatformintegration.h @@ -171,6 +171,7 @@ public: protected: void screenAdded(QPlatformScreen *screen); + void destroyScreen(QPlatformScreen *screen); }; QT_END_NAMESPACE diff --git a/src/gui/kernel/qplatformscreen.cpp b/src/gui/kernel/qplatformscreen.cpp index 71710d1cb1..fa6d785b04 100644 --- a/src/gui/kernel/qplatformscreen.cpp +++ b/src/gui/kernel/qplatformscreen.cpp @@ -52,9 +52,11 @@ QPlatformScreen::QPlatformScreen() QPlatformScreen::~QPlatformScreen() { Q_D(QPlatformScreen); - - QGuiApplicationPrivate::screen_list.removeOne(d->screen); - delete d->screen; + if (d->screen) { + qWarning("Manually deleting a QPlatformScreen. Call QPlatformIntegration::destroyScreen instead."); + QGuiApplicationPrivate::screen_list.removeOne(d->screen); + delete d->screen; + } } /*! diff --git a/src/plugins/platforms/cocoa/qcocoaintegration.mm b/src/plugins/platforms/cocoa/qcocoaintegration.mm index 72bd09625a..180cb23669 100644 --- a/src/plugins/platforms/cocoa/qcocoaintegration.mm +++ b/src/plugins/platforms/cocoa/qcocoaintegration.mm @@ -337,7 +337,7 @@ QCocoaIntegration::~QCocoaIntegration() // Delete screens in reverse order to avoid crash in case of multiple screens while (!mScreens.isEmpty()) { - delete mScreens.takeLast(); + destroyScreen(mScreens.takeLast()); } clearToolbars(); @@ -397,7 +397,7 @@ void QCocoaIntegration::updateScreens() // Now the leftovers in remainingScreens are no longer current, so we can delete them. foreach (QCocoaScreen* screen, remainingScreens) { mScreens.removeOne(screen); - delete screen; + destroyScreen(screen); } // All screens in mScreens are siblings, because we ignored the mirrors. foreach (QCocoaScreen* screen, mScreens) diff --git a/src/plugins/platforms/ios/qiosintegration.mm b/src/plugins/platforms/ios/qiosintegration.mm index 461f160892..ff4b753cc1 100644 --- a/src/plugins/platforms/ios/qiosintegration.mm +++ b/src/plugins/platforms/ios/qiosintegration.mm @@ -120,7 +120,7 @@ QIOSIntegration::~QIOSIntegration() m_inputContext = 0; foreach (QScreen *screen, QGuiApplication::screens()) - delete screen->handle(); + destroyScreen(screen->handle()); delete m_platformServices; m_platformServices = 0; diff --git a/src/plugins/platforms/kms/qkmsintegration.cpp b/src/plugins/platforms/kms/qkmsintegration.cpp index d94d7d9aaa..5ad58ba54f 100644 --- a/src/plugins/platforms/kms/qkmsintegration.cpp +++ b/src/plugins/platforms/kms/qkmsintegration.cpp @@ -74,7 +74,7 @@ QKmsIntegration::~QKmsIntegration() delete device; } foreach (QPlatformScreen *screen, m_screens) { - delete screen; + destroyScreen(screen); } delete m_fontDatabase; delete m_vtHandler; diff --git a/src/plugins/platforms/linuxfb/qlinuxfbintegration.cpp b/src/plugins/platforms/linuxfb/qlinuxfbintegration.cpp index cb870847f2..b0d99e80c1 100644 --- a/src/plugins/platforms/linuxfb/qlinuxfbintegration.cpp +++ b/src/plugins/platforms/linuxfb/qlinuxfbintegration.cpp @@ -57,7 +57,7 @@ QLinuxFbIntegration::QLinuxFbIntegration(const QStringList ¶mList) QLinuxFbIntegration::~QLinuxFbIntegration() { - delete m_primaryScreen; + destroyScreen(m_primaryScreen); } void QLinuxFbIntegration::initialize() diff --git a/src/plugins/platforms/minimalegl/qminimaleglintegration.cpp b/src/plugins/platforms/minimalegl/qminimaleglintegration.cpp index 0b12e62cc1..3fbed1ec26 100644 --- a/src/plugins/platforms/minimalegl/qminimaleglintegration.cpp +++ b/src/plugins/platforms/minimalegl/qminimaleglintegration.cpp @@ -60,7 +60,7 @@ QMinimalEglIntegration::QMinimalEglIntegration() QMinimalEglIntegration::~QMinimalEglIntegration() { - delete mScreen; + destroyScreen(mScreen); } bool QMinimalEglIntegration::hasCapability(QPlatformIntegration::Capability cap) const diff --git a/src/plugins/platforms/openwfd/qopenwfdintegration.cpp b/src/plugins/platforms/openwfd/qopenwfdintegration.cpp index 1e29fcc9b1..26bdd14327 100644 --- a/src/plugins/platforms/openwfd/qopenwfdintegration.cpp +++ b/src/plugins/platforms/openwfd/qopenwfdintegration.cpp @@ -133,3 +133,8 @@ void QOpenWFDIntegration::addScreen(QOpenWFDScreen *screen) { screenAdded(screen); } + +void QOpenWFDIntegration::destroyScreen(QOpenWFDScreen *screen) +{ + QPlatformIntegration::destroyScreen(screen); +} diff --git a/src/plugins/platforms/openwfd/qopenwfdintegration.h b/src/plugins/platforms/openwfd/qopenwfdintegration.h index 6c086b73be..9243205caa 100644 --- a/src/plugins/platforms/openwfd/qopenwfdintegration.h +++ b/src/plugins/platforms/openwfd/qopenwfdintegration.h @@ -63,6 +63,7 @@ public: QPlatformPrinterSupport *printerSupport() const; void addScreen(QOpenWFDScreen *screen); + void destroyScreen(QOpenWFDScreen *screen); private: QList mScreens; QListmDevices; diff --git a/src/plugins/platforms/openwfd/qopenwfdport.cpp b/src/plugins/platforms/openwfd/qopenwfdport.cpp index 0bdc6b2d4b..b643644800 100644 --- a/src/plugins/platforms/openwfd/qopenwfdport.cpp +++ b/src/plugins/platforms/openwfd/qopenwfdport.cpp @@ -140,7 +140,7 @@ void QOpenWFDPort::detach() mAttached = false; mOn = false; - delete mScreen; + mDevice->integration()->destroyScreen(mScreen); wfdDestroyPipeline(mDevice->handle(),mPipeline); mPipelineId = WFD_INVALID_PIPELINE_ID; diff --git a/src/plugins/platforms/qnx/qqnxintegration.cpp b/src/plugins/platforms/qnx/qqnxintegration.cpp index 6a3cd902af..dba4ba67a8 100644 --- a/src/plugins/platforms/qnx/qqnxintegration.cpp +++ b/src/plugins/platforms/qnx/qqnxintegration.cpp @@ -554,7 +554,7 @@ void QQnxIntegration::removeDisplay(QQnxScreen *screen) Q_CHECK_PTR(screen); Q_ASSERT(m_screens.contains(screen)); m_screens.removeAll(screen); - screen->deleteLater(); + destroyScreen(screen); } void QQnxIntegration::destroyDisplays() diff --git a/src/plugins/platforms/windows/qwindowsintegration.h b/src/plugins/platforms/windows/qwindowsintegration.h index d1617eaa3c..7fb37bc1f1 100644 --- a/src/plugins/platforms/windows/qwindowsintegration.h +++ b/src/plugins/platforms/windows/qwindowsintegration.h @@ -94,6 +94,7 @@ public: static QWindowsIntegration *instance(); inline void emitScreenAdded(QPlatformScreen *s) { screenAdded(s); } + inline void emitDestroyScreen(QPlatformScreen *s) { destroyScreen(s); } unsigned options() const; diff --git a/src/plugins/platforms/windows/qwindowsscreen.cpp b/src/plugins/platforms/windows/qwindowsscreen.cpp index fd57d9ee61..79219e361a 100644 --- a/src/plugins/platforms/windows/qwindowsscreen.cpp +++ b/src/plugins/platforms/windows/qwindowsscreen.cpp @@ -462,7 +462,7 @@ void QWindowsScreenManager::removeScreen(int index) if (movedWindowCount) QWindowSystemInterface::flushWindowSystemEvents(); } - delete m_screens.takeAt(index); + QWindowsIntegration::instance()->emitDestroyScreen(m_screens.takeAt(index)); } /*! @@ -497,4 +497,11 @@ bool QWindowsScreenManager::handleScreenChanges() return true; } +void QWindowsScreenManager::clearScreens() +{ + // Delete screens in reverse order to avoid crash in case of multiple screens + while (!m_screens.isEmpty()) + QWindowsIntegration::instance()->emitDestroyScreen(m_screens.takeLast()); +} + QT_END_NAMESPACE diff --git a/src/plugins/platforms/windows/qwindowsscreen.h b/src/plugins/platforms/windows/qwindowsscreen.h index aa1408358b..924912dbc2 100644 --- a/src/plugins/platforms/windows/qwindowsscreen.h +++ b/src/plugins/platforms/windows/qwindowsscreen.h @@ -127,11 +127,7 @@ public: QWindowsScreenManager(); - inline void clearScreens() { - // Delete screens in reverse order to avoid crash in case of multiple screens - while (!m_screens.isEmpty()) - delete m_screens.takeLast(); - } + void clearScreens(); bool handleScreenChanges(); bool handleDisplayChange(WPARAM wParam, LPARAM lParam); diff --git a/src/plugins/platforms/xcb/qxcbconnection.cpp b/src/plugins/platforms/xcb/qxcbconnection.cpp index 2e429939ba..08b414e82e 100644 --- a/src/plugins/platforms/xcb/qxcbconnection.cpp +++ b/src/plugins/platforms/xcb/qxcbconnection.cpp @@ -240,11 +240,12 @@ void QXcbConnection::updateScreens() ++xcbScreenNumber; } // for each xcb screen + QXcbIntegration *integration = static_cast(QGuiApplicationPrivate::platformIntegration()); // Now activeScreens is the complete set of screens which are active at this time. // Delete any existing screens which are not in activeScreens for (int i = m_screens.count() - 1; i >= 0; --i) { if (!activeScreens.contains(m_screens[i])) { - delete m_screens[i]; + integration->destroyScreen(m_screens.at(i)); m_screens.removeAt(i); } } @@ -261,7 +262,7 @@ void QXcbConnection::updateScreens() // Now that they are in the right order, emit the added signals for new screens only foreach (QXcbScreen* screen, m_screens) if (newScreens.contains(screen)) - ((QXcbIntegration*)QGuiApplicationPrivate::platformIntegration())->screenAdded(screen); + integration->screenAdded(screen); } QXcbConnection::QXcbConnection(QXcbNativeInterface *nativeInterface, bool canGrabServer, const char *displayName) @@ -400,9 +401,10 @@ QXcbConnection::~QXcbConnection() delete m_reader; + QXcbIntegration *integration = static_cast(QGuiApplicationPrivate::platformIntegration()); // Delete screens in reverse order to avoid crash in case of multiple screens while (!m_screens.isEmpty()) - delete m_screens.takeLast(); + integration->destroyScreen(m_screens.takeLast()); #ifdef XCB_USE_XLIB XCloseDisplay((Display *)m_xlib_display); -- cgit v1.2.3