diff options
author | Ivan Solovev <ivan.solovev@qt.io> | 2022-06-13 10:41:15 +0200 |
---|---|---|
committer | Ivan Solovev <ivan.solovev@qt.io> | 2022-06-13 10:52:12 +0200 |
commit | f3a306a30fc4f40d1c96fee0ed44517fe8b43d76 (patch) | |
tree | 847949a9a535464ec5d3a575ecb557a32ff21ec7 | |
parent | bb05a26d52c834cc7f3c549f3e5d66f76baf42a2 (diff) |
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
Pick-to: 6.4 6.3 6.2
Change-Id: Iad3c14ea5f0d3f3f3f9d483a1e6ab3e8b3c6c573
Reviewed-by: Jörg Bornemann <joerg.bornemann@qt.io>
-rw-r--r-- | src/serialport/qwinoverlappedionotifier.cpp | 24 |
1 files changed, 21 insertions, 3 deletions
diff --git a/src/serialport/qwinoverlappedionotifier.cpp b/src/serialport/qwinoverlappedionotifier.cpp index d03b52ae..65d63bba 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() |