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 <joerg.bornemann@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

2 files changed, 9 insertions, 8 deletions

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<MSG> 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<QWinEventNotifier *> winEventNotifierList;
-    bool winEventNotifierListModified = false;
     void activateEventNotifier(QWinEventNotifier * wen);
 
     QList<MSG> queuedUserInputEvents;