summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorIvan Solovev <ivan.solovev@qt.io>2022-06-13 10:41:15 +0200
committerIvan Solovev <ivan.solovev@qt.io>2022-06-13 10:52:12 +0200
commitf3a306a30fc4f40d1c96fee0ed44517fe8b43d76 (patch)
tree847949a9a535464ec5d3a575ecb557a32ff21ec7
parentbb05a26d52c834cc7f3c549f3e5d66f76baf42a2 (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.cpp24
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()