QCocoaEventDispatcher: make 'interrupt' work

even if we are currently inside processEvents (apparently called manually
and not from QEventLoop::exec()). A carefully crafted application
(see, for example, the linked QTBUG or even updated auto-test)
can trigger itself into failing to exit the current (potentially nested)
event loop. We can harden our Cocoa event dispatcher to detect
such condition and properly propagate 'interrupt' to where it'll
do its job, indeed, interrupting the real event loop (aka [NSApp run]).
This mainly means we have to undo what bool blocker would erroneously do.
Also, long live (as people love to say these days) to another tricky
(somewhat) auto-test (surely, it's not flaky!).

Fixes: QTBUG-79477
Change-Id: I794f0cda23e24d36be67f2bb63d52b74be057c31
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

src/plugins/platforms/cocoa/qcocoaeventdispatcher.h

@@ -186,6 +186,7 @@ public:
     QAtomicInt serialNumber;
     int lastSerial;
     bool interrupt;
+    bool propagateInterrupt = false;
 
     static void postedEventsSourceCallback(void *info);
     static void waitingObserverCallback(CFRunLoopObserverRef observer,

src/plugins/platforms/cocoa/qcocoaeventdispatcher.mm

@@ -84,13 +84,12 @@
 #include "private/qthread_p.h"
 #include "private/qguiapplication_p.h"
 #include <qdebug.h>
+#include <qscopeguard.h>
 
 #include <AppKit/AppKit.h>
 
 QT_BEGIN_NAMESPACE
 
-QT_USE_NAMESPACE
-
 static inline CFRunLoopRef mainRunLoop()
 {
     return CFRunLoopGetMain();
@@ -348,6 +347,16 @@ static inline void qt_mac_waitForMoreEvents(NSString *runLoopMode = NSDefaultRun
 bool QCocoaEventDispatcher::processEvents(QEventLoop::ProcessEventsFlags flags)
 {
     Q_D(QCocoaEventDispatcher);
+
+    // In rare rather corner cases a user's application messes with
+    // QEventLoop::exec()/exit() and QCoreApplication::processEvents(),
+    // we have to undo what bool blocker normally does.
+    d->propagateInterrupt = false;
+    const auto boolBlockerUndo = qScopeGuard([d](){
+        if (d->propagateInterrupt)
+            d->interrupt = true;
+        d->propagateInterrupt = false;
+    });
     QBoolBlocker interruptBlocker(d->interrupt, false);
 
     bool interruptLater = false;
@@ -496,7 +505,16 @@ bool QCocoaEventDispatcher::processEvents(QEventLoop::ProcessEventsFlags flags)
 
             if ((d->processEventsFlags & QEventLoop::EventLoopExec) == 0) {
                 // When called "manually", always process posted events and timers
+                bool oldInterrupt = d->interrupt;
                 d->processPostedEvents();
+                if (!oldInterrupt && d->interrupt && !d->currentModalSession()) {
+                    // We had direct processEvent call, coming not from QEventLoop::exec().
+                    // One of the posted events triggered an application to interrupt the loop.
+                    // But bool blocker will reset d->interrupt to false, so the real event
+                    // loop will never notice it was interrupted. Now we'll have to fix it by
+                    // enforcing the value of d->interrupt.
+                    d->propagateInterrupt = true;
+                }
                 retVal = d->processTimers() || retVal;
             }
 

tests/auto/corelib/kernel/qeventdispatcher/tst_qeventdispatcher.cpp

@@ -67,6 +67,7 @@ private slots:
     void sendPostedEvents_data();
     void sendPostedEvents();
     void processEventsOnlySendsQueuedEvents();
+    void eventLoopExit();
 };
 
 bool tst_QEventDispatcher::event(QEvent *e)
@@ -314,5 +315,49 @@ void tst_QEventDispatcher::processEventsOnlySendsQueuedEvents()
     QCOMPARE(object.eventsReceived, 4);
 }
 
+void tst_QEventDispatcher::eventLoopExit()
+{
+    // This test was inspired by QTBUG-79477. A particular
+    // implementation detail in QCocoaEventDispatcher allowed
+    // QEventLoop::exit() to fail to really exit the event loop.
+    // Thus this test is a part of the dispatcher auto-test.
+
+    // Imitates QApplication::exec():
+    QEventLoop mainLoop;
+    // The test itself is a lambda:
+    QTimer::singleShot(0, [&mainLoop]() {
+        // Two more single shots, both will be posted as events
+        // (zero timeout) and supposed to be processes by the
+        // mainLoop:
+
+        QTimer::singleShot(0, [&mainLoop]() {
+            // wakeUp triggers QCocoaEventDispatcher into incrementing
+            // its 'serialNumber':
+            mainLoop.wakeUp();
+            // QCocoaEventDispatcher::processEvents() will process
+            // posted events and execute the second lambda defined below:
+            QCoreApplication::processEvents();
+        });
+
+        QTimer::singleShot(0, [&mainLoop]() {
+            // With QCocoaEventDispatcher this is executed while in the
+            // processEvents (see above) and would fail to actually
+            // interrupt the loop.
+            mainLoop.exit();
+        });
+    });
+
+    bool timeoutObserved = false;
+    QTimer::singleShot(500, [&timeoutObserved, &mainLoop]() {
+        // In case the QEventLoop::exit above failed, we have to bail out
+        // early, not wasting time:
+        mainLoop.exit();
+        timeoutObserved = true;
+    });
+
+    mainLoop.exec();
+    QVERIFY(!timeoutObserved);
+}
+
 QTEST_MAIN(tst_QEventDispatcher)
 #include "tst_qeventdispatcher.moc"