From 063cdb9870ab8a9b0f1a3741a001c06289f02af4 Mon Sep 17 00:00:00 2001 From: Alex Trotsenko Date: Mon, 12 Apr 2021 16:31:51 +0300 Subject: [PATCH] QWindowsPipe{Reader|Writer}: restructure signals For QProcess, there is no point in suppressing recursive QWPR::readyRead() emission, as the former manages this logic itself. On top of that, the non-recursive nature of QWPR::readyRead() indirectly disallowed reading from the channels inside QProcess::waitForReadyRead(), if that is called from a slot connected to QProcess::readyRead(). QWPW had two signals, one allowing recursion and one not. This commit allows recursion of QWPR::readyRead() and QWPW::bytesWritten(), and moves recursion suppression to the higher- level classes. This makes the code more uniform and efficient, at the cost of a few duplicated lines. Change-Id: Ib20017fff4d92403d0bf2335f1622de4aa1ddcef Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qprocess_p.h | 3 ++ src/corelib/io/qprocess_win.cpp | 18 +++++++--- src/corelib/io/qwindowspipereader.cpp | 11 ++----- src/corelib/io/qwindowspipereader_p.h | 1 - src/corelib/io/qwindowspipewriter.cpp | 14 ++------ src/corelib/io/qwindowspipewriter_p.h | 2 -- src/network/socket/qlocalsocket_p.h | 6 ++++ src/network/socket/qlocalsocket_win.cpp | 33 +++++++++++++++---- .../auto/corelib/io/qprocess/tst_qprocess.cpp | 4 ++- 9 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h index 73db9423e6..f1572055f4 100644 --- a/src/corelib/io/qprocess_p.h +++ b/src/corelib/io/qprocess_p.h @@ -294,6 +294,9 @@ public: // private slots bool _q_canReadStandardOutput(); bool _q_canReadStandardError(); +#ifdef Q_OS_WIN + void _q_bytesWritten(qint64 bytes); +#endif bool _q_canWrite(); bool _q_startupNotification(); void _q_processDied(); diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp index d0e57b51fa..388e163317 100644 --- a/src/corelib/io/qprocess_win.cpp +++ b/src/corelib/io/qprocess_win.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include @@ -832,16 +833,25 @@ qint64 QProcessPrivate::pipeWriterBytesToWrite() const return stdinChannel.writer ? stdinChannel.writer->bytesToWrite() : qint64(0); } +void QProcessPrivate::_q_bytesWritten(qint64 bytes) +{ + Q_Q(QProcess); + + if (!emittedBytesWritten) { + QScopedValueRollback guard(emittedBytesWritten, true); + emit q->bytesWritten(bytes); + } + _q_canWrite(); +} + bool QProcessPrivate::writeToStdin() { Q_Q(QProcess); if (!stdinChannel.writer) { stdinChannel.writer = new QWindowsPipeWriter(stdinChannel.pipe[1], q); - QObject::connect(stdinChannel.writer, &QWindowsPipeWriter::bytesWritten, - q, &QProcess::bytesWritten); - QObjectPrivate::connect(stdinChannel.writer, &QWindowsPipeWriter::canWrite, - this, &QProcessPrivate::_q_canWrite); + QObjectPrivate::connect(stdinChannel.writer, &QWindowsPipeWriter::bytesWritten, + this, &QProcessPrivate::_q_bytesWritten); } else { if (stdinChannel.writer->isWriteOperationActive()) return true; diff --git a/src/corelib/io/qwindowspipereader.cpp b/src/corelib/io/qwindowspipereader.cpp index b8ea89b8e3..9030c3aaa1 100644 --- a/src/corelib/io/qwindowspipereader.cpp +++ b/src/corelib/io/qwindowspipereader.cpp @@ -39,7 +39,6 @@ ****************************************************************************/ #include "qwindowspipereader_p.h" -#include #include #include @@ -61,8 +60,7 @@ QWindowsPipeReader::QWindowsPipeReader(QObject *parent) readSequenceStarted(false), pipeBroken(false), readyReadPending(false), - winEventActPosted(false), - inReadyRead(false) + winEventActPosted(false) { ZeroMemory(&overlapped, sizeof(OVERLAPPED)); overlapped.hEvent = eventHandle; @@ -424,10 +422,8 @@ bool QWindowsPipeReader::consumePendingAndEmit(bool allowWinActPosting) if (state != Running) return false; - if (emitReadyRead && !inReadyRead) { - QScopedValueRollback guard(inReadyRead, true); + if (emitReadyRead) emit readyRead(); - } if (emitPipeClosed) { if (dwError != ERROR_BROKEN_PIPE && dwError != ERROR_PIPE_NOT_CONNECTED) emit winError(dwError, QLatin1String("QWindowsPipeReader::consumePendingAndEmit")); @@ -484,8 +480,7 @@ bool QWindowsPipeReader::waitForNotification(const QDeadlineTimer &deadline) /*! Waits for the completion of the asynchronous read operation. - Returns \c true, if we've emitted the readyRead signal (non-recursive case) - or readyRead will be emitted by the event loop (recursive case). + Returns \c true, if we've emitted the readyRead signal. */ bool QWindowsPipeReader::waitForReadyRead(int msecs) { diff --git a/src/corelib/io/qwindowspipereader_p.h b/src/corelib/io/qwindowspipereader_p.h index 8ae292da46..f02bf99880 100644 --- a/src/corelib/io/qwindowspipereader_p.h +++ b/src/corelib/io/qwindowspipereader_p.h @@ -125,7 +125,6 @@ private: bool pipeBroken; bool readyReadPending; bool winEventActPosted; - bool inReadyRead; }; QT_END_NAMESPACE diff --git a/src/corelib/io/qwindowspipewriter.cpp b/src/corelib/io/qwindowspipewriter.cpp index 4b075549d0..f2f4dce056 100644 --- a/src/corelib/io/qwindowspipewriter.cpp +++ b/src/corelib/io/qwindowspipewriter.cpp @@ -39,7 +39,6 @@ ****************************************************************************/ #include "qwindowspipewriter_p.h" -#include #include #include @@ -56,8 +55,7 @@ QWindowsPipeWriter::QWindowsPipeWriter(HANDLE pipeWriteEnd, QObject *parent) stopped(true), writeSequenceStarted(false), bytesWrittenPending(false), - winEventActPosted(false), - inBytesWritten(false) + winEventActPosted(false) { ZeroMemory(&overlapped, sizeof(OVERLAPPED)); overlapped.hEvent = eventHandle; @@ -290,12 +288,7 @@ bool QWindowsPipeWriter::consumePendingAndEmit(bool allowWinActPosting) if (stopped) return false; - emit canWrite(); - if (!inBytesWritten) { - QScopedValueRollback guard(inBytesWritten, true); - emit bytesWritten(numberOfBytesWritten); - } - + emit bytesWritten(numberOfBytesWritten); return true; } @@ -317,8 +310,7 @@ bool QWindowsPipeWriter::waitForNotification(const QDeadlineTimer &deadline) /*! Waits for the completion of the asynchronous write operation. - Returns \c true, if we've emitted the bytesWritten signal (non-recursive case) - or bytesWritten will be emitted by the event loop (recursive case). + Returns \c true, if we've emitted the bytesWritten signal. */ bool QWindowsPipeWriter::waitForWrite(int msecs) { diff --git a/src/corelib/io/qwindowspipewriter_p.h b/src/corelib/io/qwindowspipewriter_p.h index d33c2753a8..cb78195a85 100644 --- a/src/corelib/io/qwindowspipewriter_p.h +++ b/src/corelib/io/qwindowspipewriter_p.h @@ -77,7 +77,6 @@ public: HANDLE syncEvent() const { return syncHandle; } Q_SIGNALS: - void canWrite(); void bytesWritten(qint64 bytes); protected: @@ -104,7 +103,6 @@ private: bool writeSequenceStarted; bool bytesWrittenPending; bool winEventActPosted; - bool inBytesWritten; }; QT_END_NAMESPACE diff --git a/src/network/socket/qlocalsocket_p.h b/src/network/socket/qlocalsocket_p.h index a80c09b517..34dc37870c 100644 --- a/src/network/socket/qlocalsocket_p.h +++ b/src/network/socket/qlocalsocket_p.h @@ -133,6 +133,8 @@ public: #elif defined(Q_OS_WIN) ~QLocalSocketPrivate(); void destroyPipeHandles(); + void _q_canRead(); + void _q_bytesWritten(qint64 bytes); void _q_canWrite(); void _q_pipeClosed(); void _q_winError(ulong windowsError, const QString &function); @@ -161,6 +163,10 @@ public: QLocalSocket::LocalSocketState state; QString serverName; QString fullServerName; +#if defined(Q_OS_WIN) && !defined(QT_LOCALSOCKET_TCP) + bool emittedReadyRead; + bool emittedBytesWritten; +#endif Q_OBJECT_BINDABLE_PROPERTY(QLocalSocketPrivate, QLocalSocket::SocketOptions, socketOptions) }; diff --git a/src/network/socket/qlocalsocket_win.cpp b/src/network/socket/qlocalsocket_win.cpp index 677b431265..66eed86501 100644 --- a/src/network/socket/qlocalsocket_win.cpp +++ b/src/network/socket/qlocalsocket_win.cpp @@ -38,6 +38,7 @@ ****************************************************************************/ #include "qlocalsocket_p.h" +#include QT_BEGIN_NAMESPACE @@ -45,7 +46,8 @@ void QLocalSocketPrivate::init() { Q_Q(QLocalSocket); pipeReader = new QWindowsPipeReader(q); - q->connect(pipeReader, SIGNAL(readyRead()), SIGNAL(readyRead())); + QObjectPrivate::connect(pipeReader, &QWindowsPipeReader::readyRead, + this, &QLocalSocketPrivate::_q_canRead); q->connect(pipeReader, SIGNAL(pipeClosed()), SLOT(_q_pipeClosed()), Qt::QueuedConnection); q->connect(pipeReader, SIGNAL(winError(ulong,QString)), SLOT(_q_winError(ulong,QString))); } @@ -99,7 +101,9 @@ QLocalSocketPrivate::QLocalSocketPrivate() : QIODevicePrivate(), pipeWriter(0), pipeReader(0), error(QLocalSocket::UnknownSocketError), - state(QLocalSocket::UnconnectedState) + state(QLocalSocket::UnconnectedState), + emittedReadyRead(false), + emittedBytesWritten(false) { writeBufferChunkSize = QIODEVICE_BUFFERSIZE; } @@ -223,10 +227,8 @@ qint64 QLocalSocket::writeData(const char *data, qint64 len) d->write(data, len); if (!d->pipeWriter) { d->pipeWriter = new QWindowsPipeWriter(d->handle, this); - connect(d->pipeWriter, &QWindowsPipeWriter::bytesWritten, - this, &QLocalSocket::bytesWritten); - QObjectPrivate::connect(d->pipeWriter, &QWindowsPipeWriter::canWrite, - d, &QLocalSocketPrivate::_q_canWrite); + QObjectPrivate::connect(d->pipeWriter, &QWindowsPipeWriter::bytesWritten, + d, &QLocalSocketPrivate::_q_bytesWritten); } d->_q_canWrite(); return len; @@ -242,6 +244,15 @@ void QLocalSocket::abort() close(); } +void QLocalSocketPrivate::_q_canRead() +{ + Q_Q(QLocalSocket); + if (!emittedReadyRead) { + QScopedValueRollback guard(emittedReadyRead, true); + emit q->readyRead(); + } +} + void QLocalSocketPrivate::_q_pipeClosed() { Q_Q(QLocalSocket); @@ -359,6 +370,16 @@ bool QLocalSocket::setSocketDescriptor(qintptr socketDescriptor, return true; } +void QLocalSocketPrivate::_q_bytesWritten(qint64 bytes) +{ + Q_Q(QLocalSocket); + if (!emittedBytesWritten) { + QScopedValueRollback guard(emittedBytesWritten, true); + emit q->bytesWritten(bytes); + } + _q_canWrite(); +} + void QLocalSocketPrivate::_q_canWrite() { Q_Q(QLocalSocket); diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp index 857c24c571..035c5941c2 100644 --- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp +++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp @@ -1294,7 +1294,7 @@ void tst_QProcess::waitForReadyReadInAReadyReadSlot() QVERIFY(process.waitForFinished(5000)); QCOMPARE(process.exitStatus(), QProcess::NormalExit); QCOMPARE(process.exitCode(), 0); - QVERIFY(process.bytesAvailable() > bytesAvailable); + QVERIFY(process.bytesAvailable() >= bytesAvailable); } void tst_QProcess::waitForReadyReadInAReadyReadSlotSlot() @@ -1304,6 +1304,8 @@ void tst_QProcess::waitForReadyReadInAReadyReadSlotSlot() bytesAvailable = process->bytesAvailable(); process->write("bar", 4); QVERIFY(process->waitForReadyRead(5000)); + QVERIFY(process->bytesAvailable() > bytesAvailable); + bytesAvailable = process->bytesAvailable(); QTestEventLoop::instance().exitLoop(); }