Fix asserts and crashes in QWinEventNotifier activation loop

The backwards iteration was done under the assumption that the only
valid modification of the winEventNotifierList in a slot connected to
activated() would be the removal of the notifier itself. This is wrong.

Instead, iterate forwards, like before 85403d0a, and check the index
against the current list size in every iteration. This ensures that we
do not run out of bounds while the list is modified.

Also, retry the activation loop if the list was modified by a slot
connected to activated(). This ensures that all notifiers with signaled
handles are activated.

Task-number: QTBUG-65940
Change-Id: I25f305463b9234f391abc51fe0628d02f49b6931
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@qt.io>

1 files changed, 14 insertions, 9 deletions

diff --git a/src/corelib/kernel/qeventdispatcher_win.cpp b/src/corelib/kernel/qeventdispatcher_win.cpp
index bbd442d570..330870f219 100644
--- a/src/corelib/kernel/qeventdispatcher_win.cpp
+++ b/src/corelib/kernel/qeventdispatcher_win.cpp
@@ -905,6 +905,7 @@ bool QEventDispatcherWin32::registerEventNotifier(QWinEventNotifier *notifier)
         return true;
 
     d->winEventNotifierList.append(notifier);
+    d->winEventNotifierListModified = true;
 
     if (!d->winEventNotifierActivatedEvent)
         d->winEventNotifierActivatedEvent = CreateEvent(0, TRUE, FALSE, nullptr);
@@ -928,6 +929,7 @@ void QEventDispatcherWin32::unregisterEventNotifier(QWinEventNotifier *notifier)
     if (i == -1)
         return;
     d->winEventNotifierList.takeAt(i);
+    d->winEventNotifierListModified = true;
     QWinEventNotifierPrivate *nd = QWinEventNotifierPrivate::get(notifier);
     if (nd->waitHandle)
         nd->unregisterWaitObject();
@@ -938,16 +940,19 @@ void QEventDispatcherWin32::activateEventNotifiers()
     Q_D(QEventDispatcherWin32);
     ResetEvent(d->winEventNotifierActivatedEvent);
 
-    // Iterate backwards, because the notifier might remove itself on activate().
-    for (int i = d->winEventNotifierList.count(); --i >= 0;) {
-        QWinEventNotifier *notifier = d->winEventNotifierList.at(i);
-        QWinEventNotifierPrivate *nd = QWinEventNotifierPrivate::get(notifier);
-        if (nd->signaledCount.load() != 0) {
-            --nd->signaledCount;
-            nd->unregisterWaitObject();
-            d->activateEventNotifier(notifier);
+    // Activate signaled notifiers. Our winEventNotifierList can be modified in activation slots.
+    do {
+        d->winEventNotifierListModified = false;
+        for (int i = 0; i < d->winEventNotifierList.count(); ++i) {
+            QWinEventNotifier *notifier = d->winEventNotifierList.at(i);
+            QWinEventNotifierPrivate *nd = QWinEventNotifierPrivate::get(notifier);
+            if (nd->signaledCount.load() != 0) {
+                --nd->signaledCount;
+                nd->unregisterWaitObject();
+                d->activateEventNotifier(notifier);
+            }
         }
-    }
+    } while (d->winEventNotifierListModified);
 
     // Re-register the remaining activated notifiers.
     for (int i = 0; i < d->winEventNotifierList.count(); ++i) {