QProcess: Handle spurious socket notifications for stdout and stderr

On Unix systems where the GUI event dispatcher uses a notification
system for socket notifiers that is out of band compared to select(),
it's possible for the QSocketNotifier to activate after the pipe has
been read from. When that happened, the ioctl(2) call with FIONREAD
might return 0 bytes available, which we interpreted to mean EOF.

Instead of doing that, always try to read at least one byte and examine
the returned byte count from read(2). If it returns 0, that's a real
EOF; if it returns -1 EWOULDBLOCK, we simply ignore the situation.

That's the case on OS X: the Cocoa event dispatcher uses CFSocket to get
notifications and those use kevent (and, apparently, an auxiliary
thread) instead of an in-thread select() or poll(). That means the event
loop would activate the QSocketNotifier even though there is nothing to
be read.

Task-number: QTBUG-39488
Change-Id: I1a58b5b1db7a47034fb36a78a005ebff96290efb
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@digia.com>

3 files changed, 26 insertions, 10 deletions

diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp
index 0436aa5428..b03e96d0f6 100644
--- a/src/corelib/io/qprocess.cpp
+++ b/src/corelib/io/qprocess.cpp
@@ -1,6 +1,7 @@
 /****************************************************************************
 **
 ** Copyright (C) 2013 Digia Plc and/or its subsidiary(-ies).
+** Copyright (C) 2014 Intel Corporation
 ** Contact: http://www.qt-project.org/legal
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -907,18 +908,17 @@ bool QProcessPrivate::tryReadFromChannel(Channel *channel)
         return false;
 
     qint64 available = bytesAvailableInChannel(channel);
-    if (available == 0) {
-        if (channel->notifier)
-            channel->notifier->setEnabled(false);
-        closeChannel(channel);
-#if defined QPROCESS_DEBUG
-        qDebug("QProcessPrivate::tryReadFromChannel(%d), 0 bytes available", channel - &stdinChannel);
-#endif
-        return false;
-    }
+    if (available == 0)
+        available = 1;      // always try to read at least one byte
 
     char *ptr = channel->buffer.reserve(available);
     qint64 readBytes = readFromChannel(channel, ptr, available);
+    if (readBytes <= 0)
+        channel->buffer.chop(available);
+    if (readBytes == -2) {
+        // EWOULDBLOCK
+        return false;
+    }
     if (readBytes == -1) {
         processError = QProcess::ReadError;
         q->setErrorString(QProcess::tr("Error reading from process"));
@@ -928,6 +928,16 @@ bool QProcessPrivate::tryReadFromChannel(Channel *channel)
 #endif
         return false;
     }
+    if (readBytes == 0) {
+        // EOF
+        if (channel->notifier)
+            channel->notifier->setEnabled(false);
+        closeChannel(channel);
+#if defined QPROCESS_DEBUG
+        qDebug("QProcessPrivate::tryReadFromChannel(%d), 0 bytes available", channel - &stdinChannel);
+#endif
+        return false;
+    }
 #if defined QPROCESS_DEBUG
     qDebug("QProcessPrivate::tryReadFromChannel(%d), read %d bytes from the process' output", channel - &stdinChannel
             int(readBytes));
diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 3c1eeb5f91..2269740a2f 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -981,10 +981,14 @@ qint64 QProcessPrivate::readFromChannel(const Channel *channel, char *data, qint
     Q_ASSERT(channel->pipe[0] != INVALID_Q_PIPE);
     qint64 bytesRead = qt_safe_read(channel->pipe[0], data, maxlen);
 #if defined QPROCESS_DEBUG
+    int save_errno = errno;
     qDebug("QProcessPrivate::readFromChannel(%d, %p \"%s\", %lld) == %lld",
            channel - &stdinChannel,
            data, qt_prettyDebug(data, bytesRead, 16).constData(), maxlen, bytesRead);
+    errno = save_errno;
 #endif
+    if (bytesRead == -1 && errno == EWOULDBLOCK)
+        return -2;
     return bytesRead;
 }
 
diff --git a/src/corelib/io/qwindowspipereader.cpp b/src/corelib/io/qwindowspipereader.cpp
index 7dd2125e70..d8a3ec9b42 100644
--- a/src/corelib/io/qwindowspipereader.cpp
+++ b/src/corelib/io/qwindowspipereader.cpp
@@ -132,7 +132,7 @@ qint64 QWindowsPipeReader::bytesAvailable() const
 qint64 QWindowsPipeReader::read(char *data, qint64 maxlen)
 {
     if (pipeBroken && actualReadBufferSize == 0)
-        return -1;  // signal EOF
+        return 0;  // signal EOF
 
     qint64 readSoFar;
     // If startAsyncRead() has read data, copy it to its destination.
@@ -159,6 +159,8 @@ qint64 QWindowsPipeReader::read(char *data, qint64 maxlen)
             emitReadyReadTimer->stop();
         if (!readSequenceStarted)
             startAsyncRead();
+        if (readSoFar == 0)
+            return -2;      // signal EWOULDBLOCK
     }
 
     return readSoFar;