summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIvan Solovev <ivan.solovev@qt.io>2022-12-02 17:42:23 +0100
committerIvan Solovev <ivan.solovev@qt.io>2022-12-12 14:21:53 +0100
commitee17a51a12428924adadd16f8c2fe8246d6bcc7f (patch)
tree57c2e38842de8a4e91b02c3c1dda356a120a3cfc
parente3bb82c12f4cebf70242a17791fc570f5088f37d (diff)
downloadqtserialport-ee17a51a12428924adadd16f8c2fe8246d6bcc7f.tar.gz
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 Pick-to: 6.4 6.2 5.15 Change-Id: I0c568c37119b83aafd5f98a22703b19f37b4fbc9 Reviewed-by: Jörg Bornemann <joerg.bornemann@qt.io>
-rw-r--r--src/serialport/qwinoverlappedionotifier.cpp11
1 files changed, 10 insertions, 1 deletions
diff --git a/src/serialport/qwinoverlappedionotifier.cpp b/src/serialport/qwinoverlappedionotifier.cpp
index 06f2b61..3e85fd8 100644
--- a/src/serialport/qwinoverlappedionotifier.cpp
+++ b/src/serialport/qwinoverlappedionotifier.cpp
@@ -360,8 +360,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.
@@ -381,6 +381,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.