diff options
author | Ivan Solovev <ivan.solovev@qt.io> | 2022-12-02 17:42:23 +0100 |
---|---|---|
committer | Ivan Solovev <ivan.solovev@qt.io> | 2022-12-12 14:21:53 +0100 |
commit | ee17a51a12428924adadd16f8c2fe8246d6bcc7f (patch) | |
tree | 57c2e38842de8a4e91b02c3c1dda356a120a3cfc | |
parent | e3bb82c12f4cebf70242a17791fc570f5088f37d (diff) | |
download | qtserialport-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.cpp | 11 |
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. |