Fix QWinOverlappedIoNotifier

Commit f3a306a30fc4f40d1c96fee0ed44517fe8b43d76 started processing
the whole input queue in _q_notified() slot. The reads from the input
queue are guarded by the hSemaphore variable, and this approach results
in discrepancy between the hSemaphore value and the amount of messages
in the queue.
As a result, we sometimes could try to read from an empty queue, because
the semaphore still had a non-zero value.

This commit attempts to fix it by manually adjusting the hSemaphore
count in such scenarios, and also reorderding the mutex and semaphore
calls to make sure that the hSemaphore value is not modified from the
QWinOverlapped thread while it is decremented to match the messages
count.

This commit amends f3a306a30fc4f40d1c96fee0ed44517fe8b43d76.

Fixes: QTBUG-108450
Change-Id: I0c568c37119b83aafd5f98a22703b19f37b4fbc9
Reviewed-by: Jörg Bornemann <joerg.bornemann@qt.io>
(cherry picked from commit ee17a51a12428924adadd16f8c2fe8246d6bcc7f)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

1 files changed, 10 insertions, 1 deletions

diff --git a/src/serialport/qwinoverlappedionotifier.cpp b/src/serialport/qwinoverlappedionotifier.cpp
index 5d78cbc1..767fa290 100644
--- a/src/serialport/qwinoverlappedionotifier.cpp
+++ b/src/serialport/qwinoverlappedionotifier.cpp
@@ -396,8 +396,8 @@ void QWinOverlappedIoNotifierPrivate::notify(DWORD numberOfBytes, DWORD errorCod
     Q_Q(QWinOverlappedIoNotifier);
     WaitForSingleObject(hResultsMutex, INFINITE);
     results.enqueue(IOResult(numberOfBytes, errorCode, overlapped));
-    ReleaseMutex(hResultsMutex);
     ReleaseSemaphore(hSemaphore, 1, NULL);
+    ReleaseMutex(hResultsMutex);
     // Do not send a signal if we didn't process the previous one.
     // This is done to prevent soft memory leaks when working in a completely
     // synchronous way.
@@ -417,6 +417,15 @@ void QWinOverlappedIoNotifierPrivate::_q_notified()
         WaitForSingleObject(hResultsMutex, INFINITE);
         QQueue<IOResult> values;
         results.swap(values);
+        // Decreasing the semaphore count to keep it in sync with the number
+        // of messages in queue. As ReleaseSemaphore does not accept negative
+        // values, this is sort of a recommended way to go:
+        // https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-releasesemaphore#remarks
+        int numToDecrease = values.size() - 1;
+        while (numToDecrease > 0) {
+            WaitForSingleObject(hSemaphore, 0);
+            --numToDecrease;
+        }
         ReleaseMutex(hResultsMutex);
         // 'q' can go out of scope if the user decides to close the serial port
         // while processing some answer. So we need to guard against that.