From 6cea45cc24ff1e29ded753656b2830f0fae06742 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Wed, 15 Apr 2015 14:34:35 +0200 Subject: Doc: fix description of QWindowsPipeReader::read Change-Id: Ib34fc77cc05125cf698e255a5d80dbf83cf4575e Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qwindowspipereader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/corelib/io/qwindowspipereader.cpp') diff --git a/src/corelib/io/qwindowspipereader.cpp b/src/corelib/io/qwindowspipereader.cpp index e932fc0c02..c3159ca367 100644 --- a/src/corelib/io/qwindowspipereader.cpp +++ b/src/corelib/io/qwindowspipereader.cpp @@ -119,7 +119,7 @@ qint64 QWindowsPipeReader::bytesAvailable() const } /*! - Stops the asynchronous read sequence. + Copies at most \c{maxlen} bytes from the internal read buffer to \c{data}. */ qint64 QWindowsPipeReader::read(char *data, qint64 maxlen) { -- cgit v1.2.3 From 5fc52ba6e2a5425ea8edcef8aad80d9d20f47f9c Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Thu, 16 Apr 2015 14:13:44 +0200 Subject: QWindowsPipeReader: zero OVERLAPPED struct before every ReadFile According to the documentation we should always pass a zeroed OVERLAPPED object to ReadFile. Change-Id: I3f822af46a2c38e029e02461f706c4fd91c00c50 Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qwindowspipereader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/corelib/io/qwindowspipereader.cpp') diff --git a/src/corelib/io/qwindowspipereader.cpp b/src/corelib/io/qwindowspipereader.cpp index c3159ca367..8b1ec83833 100644 --- a/src/corelib/io/qwindowspipereader.cpp +++ b/src/corelib/io/qwindowspipereader.cpp @@ -89,7 +89,6 @@ void QWindowsPipeReader::setHandle(HANDLE hPipeReadEnd) readBuffer.clear(); actualReadBufferSize = 0; handle = hPipeReadEnd; - ZeroMemory(&overlapped, sizeof(overlapped)); pipeBroken = false; readyReadEmitted = false; if (hPipeReadEnd != INVALID_HANDLE_VALUE) { @@ -206,6 +205,7 @@ void QWindowsPipeReader::startAsyncRead() char *ptr = readBuffer.reserve(bytesToRead); readSequenceStarted = true; + ZeroMemory(&overlapped, sizeof(overlapped)); if (ReadFile(handle, ptr, bytesToRead, NULL, &overlapped)) { // We get notified by the QWinOverlappedIoNotifier - even in the synchronous case. return; -- cgit v1.2.3 From bb8f62148052e1208b4fe00a3fc6f25fa74432ac Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Thu, 16 Apr 2015 13:46:45 +0200 Subject: remove emitReadyReadTimer from QWindowsPipeReader The zero timeout singleshot timer emitReadyReadTimer was used to emit the readyRead signal via the event loop in case of a synchronous read. In that particular case, ReadFile would return successfully, and the notified slot would not be called. Now, that we use an I/O completion port, the notified slot is always called, even in the synchronous case. The emitReadyReadTimer is not needed anymore. This is also supported by the fact that the timer is immediately stopped in notified() after it was started in completeAsyncRead(). Change-Id: I93bcde5f067bf89a1d49005a3fddda4c8c8c95fc Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qwindowspipereader.cpp | 9 --------- 1 file changed, 9 deletions(-) (limited to 'src/corelib/io/qwindowspipereader.cpp') diff --git a/src/corelib/io/qwindowspipereader.cpp b/src/corelib/io/qwindowspipereader.cpp index 8b1ec83833..64f21d5a92 100644 --- a/src/corelib/io/qwindowspipereader.cpp +++ b/src/corelib/io/qwindowspipereader.cpp @@ -36,7 +36,6 @@ #include #include #include -#include QT_BEGIN_NAMESPACE @@ -46,12 +45,9 @@ QWindowsPipeReader::QWindowsPipeReader(QObject *parent) readBufferMaxSize(0), actualReadBufferSize(0), readSequenceStarted(false), - emitReadyReadTimer(new QTimer(this)), pipeBroken(false), readyReadEmitted(false) { - emitReadyReadTimer->setSingleShot(true); - connect(emitReadyReadTimer, SIGNAL(timeout()), SIGNAL(readyRead())); dataReadNotifier = new QWinOverlappedIoNotifier(this); connect(dataReadNotifier, &QWinOverlappedIoNotifier::notified, this, &QWindowsPipeReader::notified); } @@ -146,8 +142,6 @@ qint64 QWindowsPipeReader::read(char *data, qint64 maxlen) } if (!pipeBroken) { - if (!actualReadBufferSize) - emitReadyReadTimer->stop(); if (!readSequenceStarted) startAsyncRead(); if (readSoFar == 0) @@ -177,7 +171,6 @@ void QWindowsPipeReader::notified(quint32 numberOfBytesRead, quint32 errorCode, return; } startAsyncRead(); - emitReadyReadTimer->stop(); readyReadEmitted = true; emit readyRead(); } @@ -266,8 +259,6 @@ bool QWindowsPipeReader::completeAsyncRead(DWORD bytesRead, DWORD errorCode) actualReadBufferSize += bytesRead; readBuffer.truncate(actualReadBufferSize); - if (!emitReadyReadTimer->isActive()) - emitReadyReadTimer->start(); return true; } -- cgit v1.2.3 From a7f1c97d6096b7f9cc44df275b20d91cc75f7347 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Thu, 16 Apr 2015 16:18:45 +0200 Subject: inline QWindowsPipeReader::completeAsyncRead There's exactly one caller for this private method, and future code will be a bit simpler after moving the code to the calling site. Change-Id: Ibc65f91c770f9f29b317ceddb39a67d52106da33 Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qwindowspipereader.cpp | 54 +++++++++++++++-------------------- 1 file changed, 23 insertions(+), 31 deletions(-) (limited to 'src/corelib/io/qwindowspipereader.cpp') diff --git a/src/corelib/io/qwindowspipereader.cpp b/src/corelib/io/qwindowspipereader.cpp index 64f21d5a92..cc407dbed9 100644 --- a/src/corelib/io/qwindowspipereader.cpp +++ b/src/corelib/io/qwindowspipereader.cpp @@ -165,11 +165,33 @@ void QWindowsPipeReader::notified(quint32 numberOfBytesRead, quint32 errorCode, { if (&overlapped != notifiedOverlapped) return; - if (!completeAsyncRead(numberOfBytesRead, errorCode)) { + + switch (errorCode) { + case ERROR_SUCCESS: + break; + case ERROR_MORE_DATA: + // This is not an error. We're connected to a message mode + // pipe and the message didn't fit into the pipe's system + // buffer. We will read the remaining data in the next call. + break; + case ERROR_BROKEN_PIPE: + case ERROR_PIPE_NOT_CONNECTED: pipeBroken = true; + break; + default: + emit winError(errorCode, QLatin1String("QWindowsPipeReader::completeAsyncRead")); + pipeBroken = true; + break; + } + + readSequenceStarted = false; + if (pipeBroken) { emit pipeClosed(); return; } + + actualReadBufferSize += numberOfBytesRead; + readBuffer.truncate(actualReadBufferSize); startAsyncRead(); readyReadEmitted = true; emit readyRead(); @@ -232,36 +254,6 @@ void QWindowsPipeReader::startAsyncRead() } } -/*! - \internal - Sets the correct size of the read buffer after a read operation. - Returns \c false, if an error occurred or the connection dropped. - */ -bool QWindowsPipeReader::completeAsyncRead(DWORD bytesRead, DWORD errorCode) -{ - readSequenceStarted = false; - - switch (errorCode) { - case ERROR_SUCCESS: - break; - case ERROR_MORE_DATA: - // This is not an error. We're connected to a message mode - // pipe and the message didn't fit into the pipe's system - // buffer. We will read the remaining data in the next call. - break; - case ERROR_BROKEN_PIPE: - case ERROR_PIPE_NOT_CONNECTED: - return false; - default: - emit winError(errorCode, QLatin1String("QWindowsPipeReader::completeAsyncRead")); - return false; - } - - actualReadBufferSize += bytesRead; - readBuffer.truncate(actualReadBufferSize); - return true; -} - /*! \internal Returns the number of available bytes in the pipe. -- cgit v1.2.3 From 5ce567c536fde6b7cb93657d14df404f3e270119 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Wed, 15 Apr 2015 15:37:24 +0200 Subject: let QWindowsPipeReader::stop() cancel the current I/O operation In rare cases the I/O operation was still running after the destructor was running, which then modified free'd memory and caused a malformed heap. To prevent this, we ensure that QWindowsPipeReader::stop() cancels a running I/O operation and sets the readSequenceStarted flag correctly. Also, we prevent the start of a new read operation after we called stop(). Change-Id: If8a28bdf23a39a0e88c1770a6f66e2b24ea426bb Task-number: QTBUG-45601 Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qwindowspipereader.cpp | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) (limited to 'src/corelib/io/qwindowspipereader.cpp') diff --git a/src/corelib/io/qwindowspipereader.cpp b/src/corelib/io/qwindowspipereader.cpp index cc407dbed9..4ed9674f36 100644 --- a/src/corelib/io/qwindowspipereader.cpp +++ b/src/corelib/io/qwindowspipereader.cpp @@ -44,6 +44,7 @@ QWindowsPipeReader::QWindowsPipeReader(QObject *parent) handle(INVALID_HANDLE_VALUE), readBufferMaxSize(0), actualReadBufferSize(0), + stopped(true), readSequenceStarted(false), pipeBroken(false), readyReadEmitted(false) @@ -69,12 +70,7 @@ static bool qt_cancelIo(HANDLE handle, OVERLAPPED *overlapped) QWindowsPipeReader::~QWindowsPipeReader() { - if (readSequenceStarted) { - if (qt_cancelIo(handle, &overlapped)) - dataReadNotifier->waitForNotified(-1, &overlapped); - else - qErrnoWarning("QWindowsPipeReader: qt_cancelIo on handle %x failed.", handle); - } + stop(); } /*! @@ -87,6 +83,7 @@ void QWindowsPipeReader::setHandle(HANDLE hPipeReadEnd) handle = hPipeReadEnd; pipeBroken = false; readyReadEmitted = false; + stopped = false; if (hPipeReadEnd != INVALID_HANDLE_VALUE) { dataReadNotifier->setHandle(hPipeReadEnd); dataReadNotifier->setEnabled(true); @@ -95,13 +92,24 @@ void QWindowsPipeReader::setHandle(HANDLE hPipeReadEnd) /*! Stops the asynchronous read sequence. - This function assumes that the file already has been closed. - It does not cancel any I/O operation. + If the read sequence is running then the I/O operation is canceled. */ void QWindowsPipeReader::stop() { - dataReadNotifier->setEnabled(false); + stopped = true; + if (readSequenceStarted) { + if (qt_cancelIo(handle, &overlapped)) { + dataReadNotifier->waitForNotified(-1, &overlapped); + } else { + const DWORD dwError = GetLastError(); + if (dwError != ERROR_NOT_FOUND) { + qErrnoWarning(dwError, "QWindowsPipeReader: qt_cancelIo on handle %x failed.", + handle); + } + } + } readSequenceStarted = false; + dataReadNotifier->setEnabled(false); handle = INVALID_HANDLE_VALUE; } @@ -142,7 +150,7 @@ qint64 QWindowsPipeReader::read(char *data, qint64 maxlen) } if (!pipeBroken) { - if (!readSequenceStarted) + if (!readSequenceStarted && !stopped) startAsyncRead(); if (readSoFar == 0) return -2; // signal EWOULDBLOCK @@ -185,6 +193,13 @@ void QWindowsPipeReader::notified(quint32 numberOfBytesRead, quint32 errorCode, } readSequenceStarted = false; + + // After the reader was stopped, the only reason why this function can be called is the + // completion of a cancellation. No signals should be emitted, and no new read sequence should + // be started in this case. + if (stopped) + return; + if (pipeBroken) { emit pipeClosed(); return; -- cgit v1.2.3