From 64d949207686a0225a78de572548a5361e340ae3 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 30 Jul 2019 08:54:30 -0700 Subject: [PATCH 1/5] Fix race condition on qt_create_tls() on Windows If this function is called by multiple threads, more than one could reach the mutex locking and call TlsAlloc(), but only the last one would save the data. The others would be leaked and, worse, be used by those other threads. [ChangeLog][QtCore][QObject] Fixed a resource leak caused by a race condition if multiple QObjects were created at the same time, for the first time in an application, from multiple threads (implies threads not started with QThread). Fixes: QTBUG-77238 Change-Id: Ife213d861bb14c1787e1fffd15b63a5818bcc807 Reviewed-by: Marc Mutz Reviewed-by: Olivier Goffart (Woboq GmbH) --- src/corelib/thread/qthread_win.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/corelib/thread/qthread_win.cpp b/src/corelib/thread/qthread_win.cpp index e56fe2c6ae..5c7642c26e 100644 --- a/src/corelib/thread/qthread_win.cpp +++ b/src/corelib/thread/qthread_win.cpp @@ -99,6 +99,8 @@ void qt_create_tls() return; static QBasicMutex mutex; QMutexLocker locker(&mutex); + if (qt_current_thread_data_tls_index != TLS_OUT_OF_INDEXES) + return; qt_current_thread_data_tls_index = TlsAlloc(); } From 787e498487831c55be89979824709622ba29f17c Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Mon, 24 Jun 2019 09:41:07 +0200 Subject: [PATCH 2/5] QMutexPool: fix memory order of atomic operations The array of QAtomicPointer can be initialized using relaxed stores of nullptr, since nullptr is the whole data. But once we store an actual QMutex pointer in the array, we need to publish the indirect data thus created. We did this, with testAndSetRelease(); what was missing was a corresponding acquire fence on load, without which there is no happens-before relationship between the writes performed by the QMutex ctor and the reads performed by a subsequent mutex.lock(), say, on the same data. Fix by adding acquire fences to all loads. That includes the dtor, since mutexes may have been created in different threads, and never been imported into this_thread before the dtor is running. As a drive-by, return a new'ed QMutex that was successfully installed directly to the caller, without again going through a load-acquire. Fixes: QTBUG-59164 Change-Id: Ia25d205b1127c8c4de0979cef997d1a88123c5c3 Reviewed-by: David Faure Reviewed-by: Giuseppe D'Angelo Reviewed-by: Thiago Macieira (cherry picked from commit 65b8f59e045bb41fef99b1a44f462115de65064a) (cherry picked from commit da38f0d691d9d7eacfac5fbcbd47b887bd59bd39) --- src/corelib/thread/qmutexpool.cpp | 9 ++++++--- src/corelib/thread/qmutexpool_p.h | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/corelib/thread/qmutexpool.cpp b/src/corelib/thread/qmutexpool.cpp index 3f9e8da942..bb063b8ab6 100644 --- a/src/corelib/thread/qmutexpool.cpp +++ b/src/corelib/thread/qmutexpool.cpp @@ -104,7 +104,7 @@ QMutexPool::QMutexPool(QMutex::RecursionMode recursionMode, int size) QMutexPool::~QMutexPool() { for (int index = 0; index < mutexes.count(); ++index) - delete mutexes[index].load(); + delete mutexes[index].loadAcquire(); } /*! @@ -129,9 +129,12 @@ QMutex *QMutexPool::createMutex(int index) { // mutex not created, create one QMutex *newMutex = new QMutex(recursionMode); - if (!mutexes[index].testAndSetRelease(0, newMutex)) + if (!mutexes[index].testAndSetRelease(nullptr, newMutex)) { delete newMutex; - return mutexes[index].load(); + return mutexes[index].loadAcquire(); + } else { + return newMutex; + } } /*! diff --git a/src/corelib/thread/qmutexpool_p.h b/src/corelib/thread/qmutexpool_p.h index 89d006ac29..00710199b8 100644 --- a/src/corelib/thread/qmutexpool_p.h +++ b/src/corelib/thread/qmutexpool_p.h @@ -68,7 +68,7 @@ public: inline QMutex *get(const void *address) { int index = uint(quintptr(address)) % mutexes.count(); - QMutex *m = mutexes[index].load(); + QMutex *m = mutexes[index].loadAcquire(); if (m) return m; else From fccb519809d8e64db0e9cc54852c8612fe59b6c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Tue, 6 Aug 2019 17:48:15 +0200 Subject: [PATCH 3/5] Don't rely on QWidget::internalWinId in QWidgetBackingStore QWidget does not handle QWindow and QPlatformWindow being destroyed behind its back, and the QWidget state for internalWinId and the Qt::WA_WState_Created attribute can easily get out of sync with reality. To avoid QWidgetBackingStore mistakenly thinking that a widget does not have a platform window it can operate on we use the QWindow and QPlatformWindow handles directly, instead of relying on the winId. This is a stop gap until we can teach QWidget to deal with dynamic changes to its underlying window handles. Change-Id: Ib09bea2ad62c42e9667a20ca6b5faf0f957288da Fixes: QTBUG-74559 Reviewed-by: Lars Knoll --- src/widgets/kernel/qwidgetbackingstore.cpp | 24 ++++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/widgets/kernel/qwidgetbackingstore.cpp b/src/widgets/kernel/qwidgetbackingstore.cpp index a32eb2a03b..24b8665013 100644 --- a/src/widgets/kernel/qwidgetbackingstore.cpp +++ b/src/widgets/kernel/qwidgetbackingstore.cpp @@ -76,6 +76,11 @@ extern QRegion qt_dirtyRegion(QWidget *); Q_GLOBAL_STATIC(QPlatformTextureList, qt_dummy_platformTextureList) #endif +static bool hasPlatformWindow(QWidget *widget) +{ + return widget && widget->windowHandle() && widget->windowHandle()->handle(); +} + /** * Flushes the contents of the \a backingStore into the screen area of \a widget. * \a region is the region to be updated in \a widget coordinates. @@ -198,7 +203,7 @@ void QWidgetBackingStore::showYellowThing(QWidget *widget, const QRegion &toBePa QRegion paintRegion = toBePainted; QRect widgetRect = widget->rect(); - if (!widget->internalWinId()) { + if (!hasPlatformWindow(widget)) { QWidget *nativeParent = widget->nativeParentWidget(); const QPoint offset = widget->mapTo(nativeParent, QPoint(0, 0)); paintRegion.translate(offset); @@ -715,7 +720,7 @@ void QWidgetBackingStore::markDirtyOnScreen(const QRegion ®ion, QWidget *widg } // Alien widgets. - if (!widget->internalWinId() && !widget->isWindow()) { + if (!hasPlatformWindow(widget) && !widget->isWindow()) { QWidget *nativeParent = widget->nativeParentWidget(); // Alien widgets with the top-level as the native parent (common case). if (nativeParent == tlw) { if (!widget->testAttribute(Qt::WA_WState_InPaintEvent)) @@ -845,7 +850,7 @@ void QWidgetPrivate::moveRect(const QRect &rect, int dx, int dy) destRect = destRect.translated(dx, dy).intersected(clipR); const QRect sourceRect(destRect.translated(-dx, -dy)); const QRect parentRect(rect & clipR); - const bool nativeWithTextureChild = textureChildSeen && q->internalWinId(); + const bool nativeWithTextureChild = textureChildSeen && hasPlatformWindow(q); const bool accelerateMove = accelEnv && isOpaque && !nativeWithTextureChild #if QT_CONFIG(graphicsview) @@ -1013,9 +1018,9 @@ static void findTextureWidgetsRecursively(QWidget *tlw, QWidget *widget, QPlatfo for (int i = 0; i < wd->children.size(); ++i) { QWidget *w = qobject_cast(wd->children.at(i)); // Stop at native widgets but store them. Stop at hidden widgets too. - if (w && !w->isWindow() && w->internalWinId()) + if (w && !w->isWindow() && hasPlatformWindow(w)) nativeChildren->append(w); - if (w && !w->isWindow() && !w->internalWinId() && !w->isHidden() && QWidgetPrivate::get(w)->textureChildSeen) + if (w && !w->isWindow() && !hasPlatformWindow(w) && !w->isHidden() && QWidgetPrivate::get(w)->textureChildSeen) findTextureWidgetsRecursively(tlw, w, widgetTextures, nativeChildren); } } @@ -1046,7 +1051,7 @@ static QPlatformTextureList *widgetTexturesFor(QWidget *tlw, QWidget *widget) Q_ASSERT(!tl->isEmpty()); for (int i = 0; i < tl->count(); ++i) { QWidget *w = static_cast(tl->source(i)); - if ((w->internalWinId() && w == widget) || (!w->internalWinId() && w->nativeParentWidget() == widget)) + if ((hasPlatformWindow(w) && w == widget) || (!hasPlatformWindow(w) && w->nativeParentWidget() == widget)) return tl; } } @@ -1157,7 +1162,8 @@ void QWidgetBackingStore::sync(QWidget *exposedWidget, const QRegion &exposedReg if (!tlw->isVisible() || !tlwExtra || tlwExtra->inTopLevelResize) return; - if (!exposedWidget || !exposedWidget->internalWinId() || !exposedWidget->isVisible() || !exposedWidget->testAttribute(Qt::WA_Mapped) + if (!exposedWidget || !hasPlatformWindow(exposedWidget) + || !exposedWidget->isVisible() || !exposedWidget->testAttribute(Qt::WA_Mapped) || !exposedWidget->updatesEnabled() || exposedRegion.isEmpty()) { return; } @@ -1330,8 +1336,8 @@ void QWidgetBackingStore::doSync() w->d_func()->sendPaintEvent(w->rect()); if (w != tlw) { QWidget *npw = w->nativeParentWidget(); - if (w->internalWinId() || (npw && npw != tlw)) { - if (!w->internalWinId()) + if (hasPlatformWindow(w) || (npw && npw != tlw)) { + if (!hasPlatformWindow(w)) w = npw; QWidgetPrivate *wPrivate = w->d_func(); if (!wPrivate->needsFlush) From 53a6f7b7836ef5084106ed63f6745c20d663affa Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Tue, 6 Aug 2019 13:15:35 +0200 Subject: [PATCH 4/5] eglfs: Fix raster windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also sanitize the initial WebAssembly hack. Both eglfs and wasm lack the concept of true raster windows. A QWindow with RasterSurface is rendered with OpenGL no matter what. The two platforms took two different approaches to work around the rest of the machinery: - wasm disabled the QOpenGLContext warning for non-OpenGL QWindows, - eglfs forced the QWindow surfaceType to OpenGLSurface whenever it was originally set to RasterSurface. Now, the latter breaks since c4e9eabc309a275efc222f4127f31ba4677259b7, leaving all raster window applications failing on eglfs, because flush in the backingstore is now checking the surface type and disallows OpenGLSurface windows. (just like how QOpenGLContext disallows RasterSurface windows) To solve all this correctly, introduce a new platform capability, OpenGLOnRasterSurface, and remove the special handling in the platform plugins. Change-Id: I7785dfb1c955577bbdccdc14ebaaac5babdec57c Fixes: QTBUG-77100 Reviewed-by: Tor Arne Vestbø --- src/gui/kernel/qopenglcontext.cpp | 3 --- src/gui/kernel/qplatformintegration.cpp | 3 +++ src/gui/kernel/qplatformintegration.h | 3 ++- src/gui/kernel/qsurface.cpp | 6 ++++++ src/plugins/platforms/eglfs/api/qeglfsintegration.cpp | 1 + src/plugins/platforms/eglfs/api/qeglfswindow.cpp | 9 ++------- src/plugins/platforms/eglfs/api/qeglfswindow_p.h | 1 - src/plugins/platforms/wasm/qwasmintegration.cpp | 1 + 8 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/gui/kernel/qopenglcontext.cpp b/src/gui/kernel/qopenglcontext.cpp index 499d16c109..e8c64bdc01 100644 --- a/src/gui/kernel/qopenglcontext.cpp +++ b/src/gui/kernel/qopenglcontext.cpp @@ -977,11 +977,8 @@ bool QOpenGLContext::makeCurrent(QSurface *surface) if (!surface->surfaceHandle()) return false; if (!surface->supportsOpenGL()) { -#ifndef Q_OS_WASM // ### work around the WASM platform plugin using QOpenGLContext with raster surfaces. - // see QTBUG-70076 qWarning() << "QOpenGLContext::makeCurrent() called with non-opengl surface" << surface; return false; -#endif } if (!d->platformGLContext->makeCurrent(surface->surfaceHandle())) diff --git a/src/gui/kernel/qplatformintegration.cpp b/src/gui/kernel/qplatformintegration.cpp index ac4d12b024..258d214c7a 100644 --- a/src/gui/kernel/qplatformintegration.cpp +++ b/src/gui/kernel/qplatformintegration.cpp @@ -244,6 +244,9 @@ QPlatformServices *QPlatformIntegration::services() const \value TopStackedNativeChildWindows The platform supports native child windows via QWindowContainer without having to punch a transparent hole in the backingstore. (since 5.10) + + \value OpenGLOnRasterSurface The platform supports making a QOpenGLContext current + in combination with a QWindow of type RasterSurface. */ /*! diff --git a/src/gui/kernel/qplatformintegration.h b/src/gui/kernel/qplatformintegration.h index 1179daeb32..0b999de264 100644 --- a/src/gui/kernel/qplatformintegration.h +++ b/src/gui/kernel/qplatformintegration.h @@ -103,7 +103,8 @@ public: AllGLFunctionsQueryable, ApplicationIcon, SwitchableWidgetComposition, - TopStackedNativeChildWindows + TopStackedNativeChildWindows, + OpenGLOnRasterSurface }; virtual ~QPlatformIntegration() { } diff --git a/src/gui/kernel/qsurface.cpp b/src/gui/kernel/qsurface.cpp index 415e64b39c..709f28d431 100644 --- a/src/gui/kernel/qsurface.cpp +++ b/src/gui/kernel/qsurface.cpp @@ -39,6 +39,8 @@ #include "qsurface.h" #include "qopenglcontext.h" +#include +#include QT_BEGIN_NAMESPACE @@ -103,6 +105,10 @@ QT_BEGIN_NAMESPACE bool QSurface::supportsOpenGL() const { SurfaceType type = surfaceType(); + if (type == RasterSurface) { + QPlatformIntegration *integ = QGuiApplicationPrivate::instance()->platformIntegration(); + return integ->hasCapability(QPlatformIntegration::OpenGLOnRasterSurface); + } return type == OpenGLSurface || type == RasterGLSurface; } diff --git a/src/plugins/platforms/eglfs/api/qeglfsintegration.cpp b/src/plugins/platforms/eglfs/api/qeglfsintegration.cpp index c8a1ddf9b9..674f579b4f 100644 --- a/src/plugins/platforms/eglfs/api/qeglfsintegration.cpp +++ b/src/plugins/platforms/eglfs/api/qeglfsintegration.cpp @@ -265,6 +265,7 @@ bool QEglFSIntegration::hasCapability(QPlatformIntegration::Capability cap) cons case RasterGLSurface: return false; #endif case WindowManagement: return false; + case OpenGLOnRasterSurface: return true; default: return QPlatformIntegration::hasCapability(cap); } } diff --git a/src/plugins/platforms/eglfs/api/qeglfswindow.cpp b/src/plugins/platforms/eglfs/api/qeglfswindow.cpp index c1d5af47aa..1fed182882 100644 --- a/src/plugins/platforms/eglfs/api/qeglfswindow.cpp +++ b/src/plugins/platforms/eglfs/api/qeglfswindow.cpp @@ -64,7 +64,6 @@ QEglFSWindow::QEglFSWindow(QWindow *w) m_backingStore(0), m_rasterCompositingContext(0), #endif - m_raster(false), m_winId(0), m_surface(EGL_NO_SURFACE), m_window(0), @@ -94,11 +93,6 @@ void QEglFSWindow::create() m_winId = newWId(); - // Save the original surface type before changing to OpenGLSurface. - m_raster = (window()->surfaceType() == QSurface::RasterSurface); - if (m_raster) // change to OpenGL, but not for RasterGLSurface - window()->setSurfaceType(QSurface::OpenGLSurface); - if (window()->type() == Qt::Desktop) { QRect fullscreenRect(QPoint(), screen()->availableGeometry().size()); QWindowSystemInterface::handleGeometryChange(window(), fullscreenRect); @@ -329,7 +323,8 @@ QEglFSScreen *QEglFSWindow::screen() const bool QEglFSWindow::isRaster() const { - return m_raster || window()->surfaceType() == QSurface::RasterGLSurface; + const QWindow::SurfaceType type = window()->surfaceType(); + return type == QSurface::RasterSurface || type == QSurface::RasterGLSurface; } #ifndef QT_NO_OPENGL diff --git a/src/plugins/platforms/eglfs/api/qeglfswindow_p.h b/src/plugins/platforms/eglfs/api/qeglfswindow_p.h index b0091e2a62..be2a0630d3 100644 --- a/src/plugins/platforms/eglfs/api/qeglfswindow_p.h +++ b/src/plugins/platforms/eglfs/api/qeglfswindow_p.h @@ -118,7 +118,6 @@ protected: QOpenGLCompositorBackingStore *m_backingStore; QOpenGLContext *m_rasterCompositingContext; #endif - bool m_raster; WId m_winId; EGLSurface m_surface; diff --git a/src/plugins/platforms/wasm/qwasmintegration.cpp b/src/plugins/platforms/wasm/qwasmintegration.cpp index 3829043d07..05d6ae21f5 100644 --- a/src/plugins/platforms/wasm/qwasmintegration.cpp +++ b/src/plugins/platforms/wasm/qwasmintegration.cpp @@ -113,6 +113,7 @@ bool QWasmIntegration::hasCapability(QPlatformIntegration::Capability cap) const case RasterGLSurface: return false; // to enable this you need to fix qopenglwidget and quickwidget for wasm case MultipleWindows: return true; case WindowManagement: return true; + case OpenGLOnRasterSurface: return true; default: return QPlatformIntegration::hasCapability(cap); } } From 36cc171b9314bf77fc84d4273dceb6264aef7134 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Tue, 6 Aug 2019 07:50:14 +0200 Subject: [PATCH 5/5] tst_http2::connectToHost - fix flakiness some assumptions were incorrect: our test server immediately sends its SETTINGS frame, as a result we have to reply with client preface + SETTINGS(ACK). So QVERIFY(!prefaceOK) was wrong from the beginning and was only passing by pure luck. Change-Id: Ie43f0d4ac41deb0e5339badaae6149a9b2f9d9b3 Reviewed-by: Timur Pocheptsov Reviewed-by: Volker Hilsheimer --- tests/auto/network/access/http2/BLACKLIST | 3 --- tests/auto/network/access/http2/tst_http2.cpp | 3 --- 2 files changed, 6 deletions(-) delete mode 100644 tests/auto/network/access/http2/BLACKLIST diff --git a/tests/auto/network/access/http2/BLACKLIST b/tests/auto/network/access/http2/BLACKLIST deleted file mode 100644 index 8f26c5b89e..0000000000 --- a/tests/auto/network/access/http2/BLACKLIST +++ /dev/null @@ -1,3 +0,0 @@ - See qtbase/src/testlib/qtestblacklist.cpp for format -[connectToHost] -* diff --git a/tests/auto/network/access/http2/tst_http2.cpp b/tests/auto/network/access/http2/tst_http2.cpp index 10dad25337..4b4b8d541a 100644 --- a/tests/auto/network/access/http2/tst_http2.cpp +++ b/tests/auto/network/access/http2/tst_http2.cpp @@ -648,9 +648,6 @@ void tst_Http2::connectToHost() eventLoop.exitLoop(); QCOMPARE(reply->error(), QNetworkReply::NoError); QVERIFY(reply->isFinished()); - // Nothing must be sent yet: - QVERIFY(!prefaceOK); - QVERIFY(!serverGotSettingsACK); // Nothing received back: QVERIFY(reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).isNull()); QCOMPARE(reply->readAll().size(), 0);