macOS: Avoid recursively updating screens when window moves screen

The QWSI APIs for reporting added or removed screens is not transactional,
so when several screens change at once Qt will see each screen change as
a separate state.

As a result, Qt, or the application itself, may react to the first of
many screen updates by moving a window to a different screen -- one
which is going to updated (removed) in the next iteration of QWSI
calls.

This caused trouble on macOS, where we use many different signals to
detect that the system has changed the screens, one of them being that
a window has been moved to a different screen.

In the scenario above, we would be in the process of updating screens
in response to the system going to sleep, which means all 3 connected
screens will be disconnected and replaced with one fake screen provided
by the system.

As we delivered the removal of the first QScreen, Qt or the application,
would respond by moving the window to one of the other two screens, which
in turn would recursively trigger another round of screen updates. This
round would then proceed to remove (and delete) all remaining QScreens.
When we then recursed back to the initial round of screen updates
we would continue iterating and operating on screens that had already
been removed, causing a crash.

Since we know that the screens will stabilize eventually, and that
QCocoaScreen has cached all info based on the displayId and NSScreen,
we can safely skip any recursive invocations of updateScreens().

Fixes: QTBUG-102021
Fixes: QTBUG-84741
Pick-to: 5.15 6.2 6.3 6.4
Invaluable-help-by: Bruno Cadoret <bruno.cadoret_1@signify.com>
Change-Id: I9ff96dbcbc6f308ad2729faf2db2de7ef08513c0
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>

1 files changed, 12 insertions, 0 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoascreen.mm b/src/plugins/platforms/cocoa/qcocoascreen.mm
index 20efa30d3b..08cc22cdf3 100644
--- a/src/plugins/platforms/cocoa/qcocoascreen.mm
+++ b/src/plugins/platforms/cocoa/qcocoascreen.mm
@@ -72,6 +72,18 @@ void QCocoaScreen::initializeScreens()
 */
 void QCocoaScreen::updateScreens()
 {
+    // Adding, updating, or removing a screen below might trigger
+    // Qt or the application to move a window to a different screen,
+    // recursing back here via QCocoaWindow::windowDidChangeScreen.
+    // The update code is not re-entrant, so bail out if we end up
+    // in this situation. The screens will stabilize eventually.
+    static bool updatingScreens = false;
+    if (updatingScreens) {
+        qCInfo(lcQpaScreen) << "Skipping screen update, already updating";
+        return;
+    }
+    QBoolBlocker recursionGuard(updatingScreens);
+
     uint32_t displayCount = 0;
     if (CGGetOnlineDisplayList(0, nullptr, &displayCount) != kCGErrorSuccess)
         qFatal("Failed to get number of online displays");