QProcess/Win: move pipe draining into QWindowsPipeReader

... where it belongs.

To avoid the loop, introduce the drainAndStop() function, which allows
QWindowsPipeReader to flush the pipe itself. It determines the number of
bytes pending and blocks until the remainder of the process output is
received.

Note that the loop in drainOutputPipes() didn't actually have to
interleave the two pipes (because we're presuming that the operations
will finish instantly), so we don't do it now. Also, the code violated
the API contract: 'true' was returned when the 'wrong' channel received
data; this is now fixed as a side effect.

Change-Id: I38ed4861a238e39e793c3716e856e5bfdeed3d74
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
This commit is contained in:
Alex Trotsenko 2021-01-15 19:39:08 +02:00
parent 5ea701337d
commit 3fc6b45cbb
4 changed files with 89 additions and 32 deletions

View File

@ -1142,9 +1142,10 @@ void QProcessPrivate::_q_processDied()
// so the data is made available before we announce death.
#ifdef Q_OS_WIN
drainOutputPipes();
#endif
#else
_q_canReadStandardOutput();
_q_canReadStandardError();
#endif
// Slots connected to signals emitted by the functions called above
// might call waitFor*(), which would synchronously reap the process.

View File

@ -642,28 +642,18 @@ bool QProcessPrivate::waitForStarted(const QDeadlineTimer &)
bool QProcessPrivate::drainOutputPipes()
{
if (!stdoutChannel.reader && !stderrChannel.reader)
return false;
bool readyReadEmitted = false;
bool someReadyReadEmitted = false;
forever {
bool readyReadEmitted = false;
bool readOperationActive = false;
if (stdoutChannel.reader) {
readyReadEmitted |= stdoutChannel.reader->waitForReadyRead(0);
readOperationActive = stdoutChannel.reader && stdoutChannel.reader->isReadOperationActive();
}
if (stderrChannel.reader) {
readyReadEmitted |= stderrChannel.reader->waitForReadyRead(0);
readOperationActive |= stderrChannel.reader && stderrChannel.reader->isReadOperationActive();
}
someReadyReadEmitted |= readyReadEmitted;
if (!readOperationActive || !readyReadEmitted)
break;
QThread::yieldCurrentThread();
if (stdoutChannel.reader) {
stdoutChannel.reader->drainAndStop();
readyReadEmitted = _q_canReadStandardOutput();
}
if (stderrChannel.reader) {
stderrChannel.reader->drainAndStop();
readyReadEmitted |= _q_canReadStandardError();
}
return someReadyReadEmitted;
return readyReadEmitted;
}
bool QProcessPrivate::waitForReadyRead(const QDeadlineTimer &deadline)

View File

@ -44,6 +44,8 @@
QT_BEGIN_NAMESPACE
static const DWORD minReadBufferSize = 4096;
QWindowsPipeReader::Overlapped::Overlapped(QWindowsPipeReader *reader)
: pipeReader(reader)
{
@ -61,7 +63,8 @@ QWindowsPipeReader::QWindowsPipeReader(QObject *parent)
overlapped(this),
readBufferMaxSize(0),
actualReadBufferSize(0),
stopped(true),
bytesPending(0),
state(Stopped),
readSequenceStarted(false),
notifiedCalled(false),
pipeBroken(false),
@ -84,6 +87,7 @@ void QWindowsPipeReader::setHandle(HANDLE hPipeReadEnd)
{
readBuffer.clear();
actualReadBufferSize = 0;
bytesPending = 0;
handle = hPipeReadEnd;
pipeBroken = false;
}
@ -94,7 +98,25 @@ void QWindowsPipeReader::setHandle(HANDLE hPipeReadEnd)
*/
void QWindowsPipeReader::stop()
{
stopped = true;
state = Stopped;
cancelAsyncRead();
}
/*!
Stops the asynchronous read sequence.
Reads all pending bytes into the internal buffer.
*/
void QWindowsPipeReader::drainAndStop()
{
state = Draining;
cancelAsyncRead();
}
/*!
Stops the asynchronous read sequence.
*/
void QWindowsPipeReader::cancelAsyncRead()
{
if (readSequenceStarted) {
if (!CancelIoEx(handle, &overlapped)) {
const DWORD dwError = GetLastError();
@ -135,7 +157,7 @@ qint64 QWindowsPipeReader::read(char *data, qint64 maxlen)
}
if (!pipeBroken) {
if (!readSequenceStarted && !stopped)
if (state == Running)
startAsyncRead();
if (readSoFar == 0)
return -2; // signal EWOULDBLOCK
@ -171,7 +193,7 @@ void QWindowsPipeReader::notified(DWORD errorCode, DWORD numberOfBytesRead)
pipeBroken = true;
break;
case ERROR_OPERATION_ABORTED:
if (stopped)
if (state != Running)
break;
Q_FALLTHROUGH();
default:
@ -183,16 +205,34 @@ void QWindowsPipeReader::notified(DWORD errorCode, DWORD numberOfBytesRead)
// 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)
if (state == Stopped)
return;
if (pipeBroken) {
emit pipeClosed();
emitPipeClosed();
return;
}
actualReadBufferSize += numberOfBytesRead;
readBuffer.truncate(actualReadBufferSize);
// Read all pending data from the pipe's buffer in 'Draining' state.
if (state == Draining) {
// Determine the number of pending bytes on the first iteration.
if (bytesPending == 0)
bytesPending = checkPipeState();
else
bytesPending -= numberOfBytesRead;
if (bytesPending == 0) // all data received
return; // unblock waitForNotification() in cancelAsyncRead()
startAsyncReadHelper(bytesPending);
if (readSequenceStarted)
notifiedCalled = false; // wait for more data
return;
}
startAsyncRead();
if (!readyReadPending) {
readyReadPending = true;
@ -202,12 +242,25 @@ void QWindowsPipeReader::notified(DWORD errorCode, DWORD numberOfBytesRead)
/*!
\internal
Reads data from the pipe into the readbuffer.
Starts an asynchronous read sequence on the pipe.
*/
void QWindowsPipeReader::startAsyncRead()
{
const DWORD minReadBufferSize = 4096;
qint64 bytesToRead = qMax(checkPipeState(), minReadBufferSize);
if (readSequenceStarted)
return;
state = Running;
startAsyncReadHelper(qMax(checkPipeState(), minReadBufferSize));
}
/*!
\internal
Starts a new read sequence.
*/
void QWindowsPipeReader::startAsyncReadHelper(qint64 bytesToRead)
{
Q_ASSERT(bytesToRead != 0);
if (pipeBroken)
return;
@ -222,7 +275,6 @@ void QWindowsPipeReader::startAsyncRead()
char *ptr = readBuffer.reserve(bytesToRead);
stopped = false;
readSequenceStarted = true;
overlapped.clear();
if (!ReadFileEx(handle, ptr, bytesToRead, &overlapped, &readFileCompleted)) {
@ -267,7 +319,7 @@ DWORD QWindowsPipeReader::checkPipeState()
return bytes;
if (!pipeBroken) {
pipeBroken = true;
emit pipeClosed();
emitPipeClosed();
}
return 0;
}
@ -299,6 +351,14 @@ void QWindowsPipeReader::emitPendingReadyRead()
}
}
void QWindowsPipeReader::emitPipeClosed()
{
// We are not allowed to emit signals in either 'Stopped'
// or 'Draining' state.
if (state == Running)
emit pipeClosed();
}
/*!
Waits for the completion of the asynchronous read operation.
Returns \c true, if we've emitted the readyRead signal (non-recursive case)

View File

@ -68,6 +68,7 @@ public:
void setHandle(HANDLE hPipeReadEnd);
void startAsyncRead();
void stop();
void drainAndStop();
void setMaxReadBufferSize(qint64 size) { readBufferMaxSize = size; }
qint64 maxReadBufferSize() const { return readBufferMaxSize; }
@ -88,12 +89,15 @@ Q_SIGNALS:
void _q_queueReadyRead(QPrivateSignal);
private:
void startAsyncReadHelper(qint64 bytesToRead);
void cancelAsyncRead();
static void CALLBACK readFileCompleted(DWORD errorCode, DWORD numberOfBytesTransfered,
OVERLAPPED *overlappedBase);
void notified(DWORD errorCode, DWORD numberOfBytesRead);
DWORD checkPipeState();
bool waitForNotification(int timeout);
void emitPendingReadyRead();
void emitPipeClosed();
class Overlapped : public OVERLAPPED
{
@ -109,7 +113,9 @@ private:
qint64 readBufferMaxSize;
QRingBuffer readBuffer;
qint64 actualReadBufferSize;
bool stopped;
qint64 bytesPending;
enum State { Stopped, Running, Draining } state;
bool readSequenceStarted;
bool notifiedCalled;
bool pipeBroken;