summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIvan Solovev <ivan.solovev@qt.io>2022-06-13 10:41:15 +0200
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2022-06-13 11:18:40 +0000
commit0f5d0b4bb2148095c85642cb890bea70d329d5aa (patch)
treec409c699a9e65d7e0c3d3a93b66236f332a6ea0a
parent70640563ac9615651a5a2cbbe06e468c9e7c4d32 (diff)
downloadqtserialport-0f5d0b4bb2148095c85642cb890bea70d329d5aa.tar.gz
Windows: fix soft memory leak in synchronous mode
QWinOverlappedIoNotifier is using an atomic 'waiting' variable to make sure that it does not emit _q_notify() signals in synchronous mode. However this does not work in practice, because a notification from the I/O completion port thread can come before we enter the blocking waiting function, and the signal will be emitted anyway. Those signals migth never processed when using synchronous read/write, because in such scenarios QSerialPort works in an infinite loop in its own thread, probably without leaving a change for proper event processing. This leads to a situation when a lot of signals are queued on the event loop, and the memory consumption grows. This patch introduces another atomic variable to make sure that a new signal is never emitted until the previous one is processed. With such approach we have maximum one signal pending on the queue. The drawback of this approach is that when working in asynchronous mode, notifications might come faster than we process them, so the processing queue might grow. To solve this, we update the code that is handles notifications in asynchronous mode to process the whole input queue instead of only one item. This commit was manually picked from 01e0371f9d69943e62619fdbfe6a9d87f2a2fa16 Fixes: QTBUG-101444 Fixes: QTBUG-103822 Fixes: QTBUG-93865 Fixes: QTBUG-91237 Fixes: QTBUG-87151 Change-Id: Iad3c14ea5f0d3f3f3f9d483a1e6ab3e8b3c6c573 Reviewed-by: Jörg Bornemann <joerg.bornemann@qt.io> (cherry picked from commit f3a306a30fc4f40d1c96fee0ed44517fe8b43d76) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--src/serialport/qwinoverlappedionotifier.cpp24
1 files changed, 21 insertions, 3 deletions
diff --git a/src/serialport/qwinoverlappedionotifier.cpp b/src/serialport/qwinoverlappedionotifier.cpp
index d03b52a..65d63bb 100644
--- a/src/serialport/qwinoverlappedionotifier.cpp
+++ b/src/serialport/qwinoverlappedionotifier.cpp
@@ -93,6 +93,7 @@ public:
HANDLE hSemaphore = nullptr;
HANDLE hResultsMutex = nullptr;
QAtomicInt waiting;
+ QAtomicInt signalSent;
QQueue<IOResult> results;
};
@@ -361,14 +362,31 @@ void QWinOverlappedIoNotifierPrivate::notify(DWORD numberOfBytes, DWORD errorCod
results.enqueue(IOResult(numberOfBytes, errorCode, overlapped));
ReleaseMutex(hResultsMutex);
ReleaseSemaphore(hSemaphore, 1, NULL);
- if (!waiting)
+ // 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.
+ if (!waiting && !signalSent.loadAcquire()) {
+ signalSent.storeRelease(1);
emit q->_q_notify();
+ }
}
void QWinOverlappedIoNotifierPrivate::_q_notified()
{
- if (WaitForSingleObject(hSemaphore, 0) == WAIT_OBJECT_0)
- dispatchNextIoResult();
+ Q_Q(QWinOverlappedIoNotifier);
+ signalSent.storeRelease(0); // signal processed - ready for a new one
+ if (WaitForSingleObject(hSemaphore, 0) == WAIT_OBJECT_0) {
+ // As we do not queue signals anymore, we need to process the whole
+ // queue at once.
+ WaitForSingleObject(hResultsMutex, INFINITE);
+ QQueue<IOResult> values;
+ results.swap(values);
+ ReleaseMutex(hResultsMutex);
+ while (!values.empty()) {
+ IOResult ioresult = values.dequeue();
+ emit q->notified(ioresult.numberOfBytes, ioresult.errorCode, ioresult.overlapped);
+ }
+ }
}
OVERLAPPED *QWinOverlappedIoNotifierPrivate::dispatchNextIoResult()