From 197029b3d23237e61311019de1b63e3ce6720ed5 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 10 Dec 2018 19:18:57 -0800 Subject: Make the QEventDispatcherWin32Private::interrupt flag atomic Not entirely sure that this solves the problem reported in the bug report, but here's the theory: the loop 633 while (!d->interrupt) { ... 710 } has few calls that recurse back, so the compiler optimizer can assume that the variable remains unchanged (not interrupted) in most of the branches. Additionally, it can assume the variable did not change from there to 712 // still nothing - wait for message or signalled objects 713 canWait = (!retVal 714 && !d->interrupt 715 && (flags & QEventLoop::WaitForMoreEvents)); Which causes canWait to be true, despite having been interrupted by another thread. Changing to an atomic does not force the reloading of the variable (strictly speaking, would need volatile, but all atomic implementations do reload now), but it solves the problem of data race, which was UB. The equivalent variable in the Unix event dispatcher is atomic (commit 49d7e71f77f899c05e4b5187e8834dfcbddf4505 from 2013). I've reordered the bool members so they're all together and reduce the amount of padding in this class. Fixes: QTBUG-72438 Change-Id: I4ac1156702324f0fb814fffd156f290df94dc4c7 Reviewed-by: Joerg Bornemann Reviewed-by: Thiago Macieira --- src/corelib/kernel/qeventdispatcher_win.cpp | 10 +++++----- src/corelib/kernel/qeventdispatcher_win_p.h | 7 ++++--- 2 files changed, 9 insertions(+), 8 deletions(-) (limited to 'src/corelib/kernel') diff --git a/src/corelib/kernel/qeventdispatcher_win.cpp b/src/corelib/kernel/qeventdispatcher_win.cpp index 0bddf89b15..b3b6b1be20 100644 --- a/src/corelib/kernel/qeventdispatcher_win.cpp +++ b/src/corelib/kernel/qeventdispatcher_win.cpp @@ -95,7 +95,7 @@ class QEventDispatcherWin32Private; LRESULT QT_WIN_CALLBACK qt_internal_proc(HWND hwnd, UINT message, WPARAM wp, LPARAM lp); QEventDispatcherWin32Private::QEventDispatcherWin32Private() - : threadId(GetCurrentThreadId()), interrupt(false), closingDown(false), internalHwnd(0), + : threadId(GetCurrentThreadId()), interrupt(false), internalHwnd(0), getMessageHook(0), serialNumber(0), lastSerialNumber(0), sendPostedEventsWindowsTimerId(0), wakeUps(0), activateNotifiersPosted(false), winEventNotifierActivatedEvent(NULL) { @@ -552,7 +552,7 @@ bool QEventDispatcherWin32::processEvents(QEventLoop::ProcessEventsFlags flags) wakeUp(); // trigger a call to sendPostedEvents() } - d->interrupt = false; + d->interrupt.store(false); emit awake(); bool canWait; @@ -568,7 +568,7 @@ bool QEventDispatcherWin32::processEvents(QEventLoop::ProcessEventsFlags flags) pHandles = &d->winEventNotifierActivatedEvent; } QVarLengthArray processedTimers; - while (!d->interrupt) { + while (!d->interrupt.load()) { MSG msg; bool haveMessage; @@ -649,7 +649,7 @@ bool QEventDispatcherWin32::processEvents(QEventLoop::ProcessEventsFlags flags) // still nothing - wait for message or signalled objects canWait = (!retVal - && !d->interrupt + && !d->interrupt.load() && (flags & QEventLoop::WaitForMoreEvents)); if (canWait) { emit aboutToBlock(); @@ -1016,7 +1016,7 @@ void QEventDispatcherWin32::wakeUp() void QEventDispatcherWin32::interrupt() { Q_D(QEventDispatcherWin32); - d->interrupt = true; + d->interrupt.store(true); wakeUp(); } diff --git a/src/corelib/kernel/qeventdispatcher_win_p.h b/src/corelib/kernel/qeventdispatcher_win_p.h index a7ed8dda8a..3bb618153b 100644 --- a/src/corelib/kernel/qeventdispatcher_win_p.h +++ b/src/corelib/kernel/qeventdispatcher_win_p.h @@ -165,8 +165,7 @@ public: DWORD threadId; - bool interrupt; - bool closingDown; + QAtomicInt interrupt; // internal window handle used for socketnotifiers/timers/etc HWND internalHwnd; @@ -193,9 +192,11 @@ public: void postActivateSocketNotifiers(); void doWsaAsyncSelect(int socket, long event); + bool closingDown = false; + + bool winEventNotifierListModified = false; HANDLE winEventNotifierActivatedEvent; QList winEventNotifierList; - bool winEventNotifierListModified = false; void activateEventNotifier(QWinEventNotifier * wen); QList queuedUserInputEvents; -- cgit v1.2.3 From 99566f68755449dbabb7345da5b5876e82f159f1 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 22 Jan 2019 10:52:57 -0800 Subject: Use qEnvironmentVariable for QT_PLUGIN_PATHS This is required for non-ANSI paths on Windows. Change-Id: Id98140e1c2f0426cabbefffd157c4065c3bdfd40 Reviewed-by: Friedemann Kleint --- src/corelib/kernel/qcoreapplication.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/corelib/kernel') diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index b6b4da3885..c637f0c1c9 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -2668,9 +2668,9 @@ QStringList QCoreApplication::libraryPaths() QStringList *app_libpaths = new QStringList; coreappdata()->app_libpaths.reset(app_libpaths); - const QByteArray libPathEnv = qgetenv("QT_PLUGIN_PATH"); + QString libPathEnv = qEnvironmentVariable("QT_PLUGIN_PATH"); if (!libPathEnv.isEmpty()) { - QStringList paths = QFile::decodeName(libPathEnv).split(QDir::listSeparator(), QString::SkipEmptyParts); + QStringList paths = libPathEnv.split(QDir::listSeparator(), QString::SkipEmptyParts); for (QStringList::const_iterator it = paths.constBegin(); it != paths.constEnd(); ++it) { QString canonicalPath = QDir(*it).canonicalPath(); if (!canonicalPath.isEmpty() -- cgit v1.2.3 From 96404f7ac89244c944adaa7533c6292e7a614311 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 2 Feb 2019 08:00:15 -0800 Subject: Doc: update the note about nested deleteLater() If you enter a nested event loop, cause a deleteLater(), exit that event loop, then enter a new one, the nesting count will be the same so those are legitimate targets for deletion. Task-number: QTBUG-73432 Change-Id: Id98140e1c2f0426cabbefffd157f975b5e291ccd Reviewed-by: Paul Wicking Reviewed-by: Thiago Macieira --- src/corelib/kernel/qobject.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/corelib/kernel') diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp index b4e885e407..cf838b6947 100644 --- a/src/corelib/kernel/qobject.cpp +++ b/src/corelib/kernel/qobject.cpp @@ -2171,8 +2171,10 @@ void QObject::removeEventFilter(QObject *obj) Note that entering and leaving a new event loop (e.g., by opening a modal dialog) will \e not perform the deferred deletion; for the object to be - deleted, the control must return to the event loop from which - deleteLater() was called. + deleted, the control must return to the event loop from which deleteLater() + was called. This does not apply to objects deleted while a previous, nested + event loop was still running: the Qt event loop will delete those objects + as soon as the new nested event loop starts. \b{Note:} It is safe to call this function more than once; when the first deferred deletion event is delivered, any pending events for the -- cgit v1.2.3