QProcess/Win: get rid of incremental wait

Introduce QProcessPoller helper class on Windows that implements
waiting for any activity from a child process.

Change-Id: I99414db4424b4342e0fa3d7a3789f85cad76d190
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

src/corelib/io/qprocess_win.cpp

@@ -85,6 +85,46 @@ QProcessEnvironment QProcessEnvironment::systemEnvironment()
 
 #if QT_CONFIG(process)
 
+namespace {
+struct QProcessPoller
+{
+    QProcessPoller(const QProcessPrivate &proc);
+
+    int poll(const QDeadlineTimer &deadline);
+
+    enum { maxHandles = 4 };
+    HANDLE handles[maxHandles];
+    DWORD handleCount = 0;
+};
+
+QProcessPoller::QProcessPoller(const QProcessPrivate &proc)
+{
+    if (proc.stdinChannel.writer)
+        handles[handleCount++] = proc.stdinChannel.writer->syncEvent();
+    if (proc.stdoutChannel.reader)
+        handles[handleCount++] = proc.stdoutChannel.reader->syncEvent();
+    if (proc.stderrChannel.reader)
+        handles[handleCount++] = proc.stderrChannel.reader->syncEvent();
+
+    handles[handleCount++] = proc.pid->hProcess;
+}
+
+int QProcessPoller::poll(const QDeadlineTimer &deadline)
+{
+    DWORD waitRet;
+
+    do {
+        waitRet = WaitForMultipleObjectsEx(handleCount, handles, FALSE,
+                                           deadline.remainingTime(), TRUE);
+    } while (waitRet == WAIT_IO_COMPLETION);
+
+    if (waitRet - WAIT_OBJECT_0 < handleCount)
+        return 1;
+
+    return (waitRet == WAIT_TIMEOUT) ? 0 : -1;
+}
+} // anonymous namespace
+
 static bool qt_create_pipe(Q_PIPE *pipe, bool isInputPipe, BOOL defInheritFlag)
 {
     // Anomymous pipes do not support asynchronous I/O. Thus we
@@ -658,16 +698,22 @@ bool QProcessPrivate::drainOutputPipes()
 
 bool QProcessPrivate::waitForReadyRead(const QDeadlineTimer &deadline)
 {
-    QIncrementalSleepTimer timer(deadline.remainingTime());
-
     forever {
         if (!writeBuffer.isEmpty() && !_q_canWrite())
             return false;
-        if (stdinChannel.writer && stdinChannel.writer->waitForWrite(0))
-            timer.resetIncrements();
 
-        if ((stdoutChannel.reader && stdoutChannel.reader->waitForReadyRead(0))
+        QProcessPoller poller(*this);
-            || (stderrChannel.reader && stderrChannel.reader->waitForReadyRead(0)))
+        int ret = poller.poll(deadline);
+        if (ret < 0)
+            return false;
+        if (ret == 0)
+            break;
+
+        if (stdinChannel.writer)
+            stdinChannel.writer->checkForWrite();
+
+        if ((stdoutChannel.reader && stdoutChannel.reader->checkForReadyRead())
+            || (stderrChannel.reader && stderrChannel.reader->checkForReadyRead()))
             return true;
 
         if (!pid)
@@ -678,10 +724,6 @@ bool QProcessPrivate::waitForReadyRead(const QDeadlineTimer &deadline)
                 processFinished();
             return readyReadEmitted;
         }
-
-        Sleep(timer.nextSleepTime());
-        if (timer.hasTimedOut())
-            break;
     }
 
     setError(QProcess::Timedout);
@@ -690,8 +732,6 @@ bool QProcessPrivate::waitForReadyRead(const QDeadlineTimer &deadline)
 
 bool QProcessPrivate::waitForBytesWritten(const QDeadlineTimer &deadline)
 {
-    QIncrementalSleepTimer timer(deadline.remainingTime());
-
     forever {
         // If no write is pending, try to start one. However, at entry into
         // the loop the write buffer can be empty to start with, in which
@@ -699,17 +739,24 @@ bool QProcessPrivate::waitForBytesWritten(const QDeadlineTimer &deadline)
         if (pipeWriterBytesToWrite() == 0 && !_q_canWrite())
             return false;
 
+        QProcessPoller poller(*this);
+        int ret = poller.poll(deadline);
+        if (ret < 0)
+            return false;
+        if (ret == 0)
+            break;
+
         Q_ASSERT(stdinChannel.writer);
-        if (stdinChannel.writer->waitForWrite(0))
+        if (stdinChannel.writer->checkForWrite())
             return true;
 
         // If we wouldn't write anything, check if we can read stdout.
-        if (stdoutChannel.reader && stdoutChannel.reader->waitForReadyRead(0))
+        if (stdoutChannel.reader)
-            timer.resetIncrements();
+            stdoutChannel.reader->checkForReadyRead();
 
         // Check if we can read stderr.
-        if (stderrChannel.reader && stderrChannel.reader->waitForReadyRead(0))
+        if (stderrChannel.reader)
-            timer.resetIncrements();
+            stderrChannel.reader->checkForReadyRead();
 
         // Check if the process died while reading.
         if (!pid)
@@ -723,10 +770,6 @@ bool QProcessPrivate::waitForBytesWritten(const QDeadlineTimer &deadline)
                 processFinished();
             return false;
         }
-
-        // Only wait for as long as we've been asked.
-        if (timer.hasTimedOut())
-            break;
     }
 
     setError(QProcess::Timedout);
@@ -739,30 +782,33 @@ bool QProcessPrivate::waitForFinished(const QDeadlineTimer &deadline)
     qDebug("QProcessPrivate::waitForFinished(%lld)", deadline.remainingTime());
 #endif
 
-    QIncrementalSleepTimer timer(deadline.remainingTime());
-
     forever {
         if (!writeBuffer.isEmpty() && !_q_canWrite())
             return false;
-        if (stdinChannel.writer && stdinChannel.writer->waitForWrite(0))
+
-            timer.resetIncrements();
+        QProcessPoller poller(*this);
-        if (stdoutChannel.reader && stdoutChannel.reader->waitForReadyRead(0))
+        int ret = poller.poll(deadline);
-            timer.resetIncrements();
+        if (ret < 0)
-        if (stderrChannel.reader && stderrChannel.reader->waitForReadyRead(0))
+            return false;
-            timer.resetIncrements();
+        if (ret == 0)
+            break;
+
+        if (stdinChannel.writer)
+            stdinChannel.writer->checkForWrite();
+        if (stdoutChannel.reader)
+            stdoutChannel.reader->checkForReadyRead();
+        if (stderrChannel.reader)
+            stderrChannel.reader->checkForReadyRead();
 
         if (!pid)
             return true;
 
-        if (WaitForSingleObject(pid->hProcess, timer.nextSleepTime()) == WAIT_OBJECT_0) {
+        if (WaitForSingleObject(pid->hProcess, 0) == WAIT_OBJECT_0) {
             drainOutputPipes();
             if (pid)
                 processFinished();
             return true;
         }
-
-        if (timer.hasTimedOut())
-            break;
     }
 
     setError(QProcess::Timedout);

src/corelib/io/qwindowspipereader.cpp

@@ -51,7 +51,7 @@ QWindowsPipeReader::QWindowsPipeReader(QObject *parent)
     : QObject(parent),
       handle(INVALID_HANDLE_VALUE),
       eventHandle(CreateEvent(NULL, FALSE, FALSE, NULL)),
-      syncHandle(CreateEvent(NULL, FALSE, FALSE, NULL)),
+      syncHandle(CreateEvent(NULL, TRUE, FALSE, NULL)),
       waitObject(NULL),
       readBufferMaxSize(0),
       actualReadBufferSize(0),
@@ -396,6 +396,7 @@ bool QWindowsPipeReader::event(QEvent *e)
  */
 bool QWindowsPipeReader::consumePendingAndEmit(bool allowWinActPosting)
 {
+    ResetEvent(syncHandle);
     mutex.lock();
 
     // Enable QEvent::WinEventAct posting.

src/corelib/io/qwindowspipereader_p.h

@@ -81,9 +81,11 @@ public:
     qint64 read(char *data, qint64 maxlen);
     bool canReadLine() const;
     bool waitForReadyRead(int msecs);
+    bool checkForReadyRead() { return consumePendingAndEmit(false); }
     bool waitForPipeClosed(int msecs);
 
     bool isReadOperationActive() const;
+    HANDLE syncEvent() const { return syncHandle; }
 
 Q_SIGNALS:
     void winError(ulong, const QString &);

src/corelib/io/qwindowspipewriter.cpp

@@ -49,7 +49,7 @@ QWindowsPipeWriter::QWindowsPipeWriter(HANDLE pipeWriteEnd, QObject *parent)
     : QObject(parent),
       handle(pipeWriteEnd),
       eventHandle(CreateEvent(NULL, FALSE, FALSE, NULL)),
-      syncHandle(CreateEvent(NULL, FALSE, FALSE, NULL)),
+      syncHandle(CreateEvent(NULL, TRUE, FALSE, NULL)),
       waitObject(NULL),
       pendingBytesWrittenValue(0),
       lastError(ERROR_SUCCESS),
@@ -267,6 +267,7 @@ bool QWindowsPipeWriter::event(QEvent *e)
  */
 bool QWindowsPipeWriter::consumePendingAndEmit(bool allowWinActPosting)
 {
+    ResetEvent(syncHandle);
     QMutexLocker locker(&mutex);
 
     // Enable QEvent::WinEventAct posting.

src/corelib/io/qwindowspipewriter_p.h

@@ -52,8 +52,6 @@
 // We mean it.
 //
 
-#include <QtCore/private/qglobal_p.h>
-#include <qelapsedtimer.h>
 #include <qobject.h>
 #include <qdeadlinetimer.h>
 #include <qmutex.h>
@@ -63,54 +61,6 @@
 
 QT_BEGIN_NAMESPACE
 
-#define SLEEPMIN 10
-#define SLEEPMAX 500
-
-class QIncrementalSleepTimer
-{
-
-public:
-    QIncrementalSleepTimer(int msecs)
-        : totalTimeOut(msecs)
-        , nextSleep(qMin(SLEEPMIN, totalTimeOut))
-    {
-        if (totalTimeOut == -1)
-            nextSleep = SLEEPMIN;
-        timer.start();
-    }
-
-    int nextSleepTime()
-    {
-        int tmp = nextSleep;
-        nextSleep = qMin(nextSleep * 2, qMin(SLEEPMAX, timeLeft()));
-        return tmp;
-    }
-
-    int timeLeft() const
-    {
-        if (totalTimeOut == -1)
-            return SLEEPMAX;
-        return qMax(int(totalTimeOut - timer.elapsed()), 0);
-    }
-
-    bool hasTimedOut() const
-    {
-        if (totalTimeOut == -1)
-            return false;
-        return timer.elapsed() >= totalTimeOut;
-    }
-
-    void resetIncrements()
-    {
-        nextSleep = qMin(SLEEPMIN, timeLeft());
-    }
-
-private:
-    QElapsedTimer timer;
-    int totalTimeOut;
-    int nextSleep;
-};
-
 class Q_CORE_EXPORT QWindowsPipeWriter : public QObject
 {
     Q_OBJECT
@@ -121,8 +71,10 @@ public:
     bool write(const QByteArray &ba);
     void stop();
     bool waitForWrite(int msecs);
+    bool checkForWrite() { return consumePendingAndEmit(false); }
     bool isWriteOperationActive() const;
     qint64 bytesToWrite() const;
+    HANDLE syncEvent() const { return syncHandle; }
 
 Q_SIGNALS:
     void canWrite();