QProcess: Ensure that the stdin buffer is cleared on start()

The buffer may have been left dirty if we were unable to write all the
data to the child process in the previous run. So ensure we clear it
before starting a new one. We already did that for stdout and stderr,
for some reason.

Task-number: QTBUG-44517
Change-Id: I1a800c709d3543699131ffff13c419da3bbffacf
Reviewed-by: Kai Koehne <kai.koehne@theqtcompany.com>
Reviewed-by: Joerg Bornemann <joerg.bornemann@theqtcompany.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>

src/corelib/io/qprocess.cpp

@@ -1,7 +1,7 @@
 /****************************************************************************
 **
 ** Copyright (C) 2015 The Qt Company Ltd.
-** Copyright (C) 2014 Intel Corporation
+** Copyright (C) 2015 Intel Corporation
 ** Contact: http://www.qt.io/licensing/
 **
 ** This file is part of the QtCore module of the Qt Toolkit.
@@ -2135,6 +2135,7 @@ void QProcessPrivate::start(QIODevice::OpenMode mode)
     qDebug() << "QProcess::start(" << program << ',' << arguments << ',' << mode << ')';
 #endif
 
+    stdinChannel.buffer.clear();
     stdoutChannel.buffer.clear();
     stderrChannel.buffer.clear();
 

src/corelib/io/qwindowspipewriter.cpp

@@ -85,7 +85,7 @@ qint64 QWindowsPipeWriter::write(const char *ptr, qint64 maxlen)
         return -1;
 
     QMutexLocker locker(&lock);
-    data.append(QByteArray(ptr, maxlen));
+    data.append(ptr, maxlen);
     waitCondition.wakeOne();
     return maxlen;
 }

tests/auto/corelib/io/qprocess/qprocess.pri

@@ -4,6 +4,7 @@ SUBPROGRAMS = \
           testProcessEcho2 \
           testProcessEcho3 \
           testProcessEnvironment \
+          testProcessHang \
           testProcessNormal \
           testProcessOutput \
           testProcessDeadWhileReading \

tests/auto/corelib/io/qprocess/testProcessHang/main.cpp

@@ -0,0 +1,62 @@
+/****************************************************************************
+**
+** Copyright (C) 2015 Intel Corporation.
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL21$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file. Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** As a special exception, The Qt Company gives you certain additional
+** rights. These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <stdio.h>
+
+#if defined(WIN32) || defined(_WIN32) || defined(__WIN32__)
+#  include <windows.h>
+
+void sleepForever()
+{
+    ::Sleep(INFINITE);
+}
+#else
+#  include <unistd.h>
+
+void sleepForever()
+{
+    ::pause();
+}
+#endif
+
+int main()
+{
+    puts("ready.");
+    fflush(stdout);
+    fprintf(stderr, "ready.\n");
+    fflush(stderr);
+
+    // sleep forever, simulating a hung application
+    sleepForever();
+    return 0;
+}

tests/auto/corelib/io/qprocess/testProcessHang/testProcessHang.pro

@@ -0,0 +1,4 @@
+SOURCES = main.cpp
+CONFIG -= qt app_bundle
+CONFIG += console
+DESTDIR = ./

tests/auto/corelib/io/qprocess/tst_qprocess.cpp

@@ -1,6 +1,7 @@
 /****************************************************************************
 **
 ** Copyright (C) 2015 The Qt Company Ltd.
+** Copyright (C) 2015 Intel Corporation.
 ** Contact: http://www.qt.io/licensing/
 **
 ** This file is part of the test suite of the Qt Toolkit.
@@ -150,6 +151,9 @@ private slots:
     void onlyOneStartedSignal();
     void finishProcessBeforeReadingDone();
     void waitForStartedWithoutStart();
+    void startStopStartStop();
+    void startStopStartStopBuffers_data();
+    void startStopStartStopBuffers();
 
     // keep these at the end, since they use lots of processes and sometimes
     // caused obscure failures to occur in tests that followed them (esp. on the Mac)
@@ -2329,12 +2333,110 @@ void tst_QProcess::finishProcessBeforeReadingDone()
     QCOMPARE(lines.last(), QStringLiteral("10239 -this is a number"));
 }
 
+//-----------------------------------------------------------------------------
 void tst_QProcess::waitForStartedWithoutStart()
 {
     QProcess process;
     QVERIFY(!process.waitForStarted(5000));
 }
 
+//-----------------------------------------------------------------------------
+void tst_QProcess::startStopStartStop()
+{
+    // we actually do start-stop x 3 :-)
+    QProcess process;
+    process.start("testProcessNormal/testProcessNormal");
+    QVERIFY(process.waitForFinished());
+    QCOMPARE(process.exitCode(), 0);
+
+    process.start("testExitCodes/testExitCodes", QStringList() << "1");
+    QVERIFY(process.waitForFinished());
+    QCOMPARE(process.exitCode(), 1);
+
+    process.start("testProcessNormal/testProcessNormal");
+    QVERIFY(process.waitForFinished());
+    QCOMPARE(process.exitCode(), 0);
+}
+
+//-----------------------------------------------------------------------------
+void tst_QProcess::startStopStartStopBuffers_data()
+{
+    QTest::addColumn<int>("channelMode1");
+    QTest::addColumn<int>("channelMode2");
+
+    QTest::newRow("separate-separate") << int(QProcess::SeparateChannels) << int(QProcess::SeparateChannels);
+    QTest::newRow("separate-merged") << int(QProcess::SeparateChannels) << int(QProcess::MergedChannels);
+    QTest::newRow("merged-separate") << int(QProcess::MergedChannels) << int(QProcess::SeparateChannels);
+    QTest::newRow("merged-merged") << int(QProcess::MergedChannels) << int(QProcess::MergedChannels);
+    QTest::newRow("merged-forwarded") << int(QProcess::MergedChannels) << int(QProcess::ForwardedChannels);
+}
+
+void tst_QProcess::startStopStartStopBuffers()
+{
+    QFETCH(int, channelMode1);
+    QFETCH(int, channelMode2);
+
+    QProcess process;
+    process.setProcessChannelMode(QProcess::ProcessChannelMode(channelMode1));
+    process.start("testProcessHang/testProcessHang");
+    QVERIFY2(process.waitForReadyRead(), process.errorString().toLocal8Bit());
+    if (channelMode1 == QProcess::SeparateChannels || channelMode1 == QProcess::ForwardedOutputChannel) {
+        process.setReadChannel(QProcess::StandardError);
+        if (process.bytesAvailable() == 0)
+            QVERIFY(process.waitForReadyRead());
+        process.setReadChannel(QProcess::StandardOutput);
+    }
+
+    // We want to test that the write buffer still has bytes after the child
+    // exiting. We do that by writing to a child process that never reads. We
+    // just have to write more data than a pipe can hold, so that even if
+    // QProcess finds the pipe writable (during waitForFinished() or in the
+    // QWindowsPipeWriter thread), some data will remain. The worst case I know
+    // of is Linux, which defaults to 64 kB of buffer.
+
+    process.write(QByteArray(128 * 1024, 'a'));
+    QVERIFY(process.bytesToWrite() > 0);
+    process.kill();
+
+    QVERIFY(process.waitForFinished());
+
+#ifndef Q_OS_WIN
+    // confirm that our buffers are still full
+    // Note: this doesn't work on Windows because our buffers are drained into
+    // QWindowsPipeWriter before being sent to the child process.
+    QVERIFY(process.bytesToWrite() > 0);
+    QVERIFY(process.bytesAvailable() > 0); // channelMode1 is not ForwardedChannels
+    if (channelMode1 == QProcess::SeparateChannels || channelMode1 == QProcess::ForwardedOutputChannel) {
+        process.setReadChannel(QProcess::StandardError);
+        QVERIFY(process.bytesAvailable() > 0);
+        process.setReadChannel(QProcess::StandardOutput);
+    }
+#endif
+
+    process.setProcessChannelMode(QProcess::ProcessChannelMode(channelMode2));
+    process.start("testProcessEcho2/testProcessEcho2", QIODevice::ReadWrite | QIODevice::Text);
+
+    // the buffers should now be empty
+    QCOMPARE(process.bytesToWrite(), qint64(0));
+    QCOMPARE(process.bytesAvailable(), qint64(0));
+    process.setReadChannel(QProcess::StandardError);
+    QCOMPARE(process.bytesAvailable(), qint64(0));
+    process.setReadChannel(QProcess::StandardOutput);
+
+    process.write("line3\n");
+    process.closeWriteChannel();
+    QVERIFY(process.waitForFinished());
+    QCOMPARE(process.exitCode(), 0);
+
+    if (channelMode2 == QProcess::MergedChannels) {
+        QCOMPARE(process.readAll(), QByteArray("lliinnee33\n\n"));
+    } else if (channelMode2 != QProcess::ForwardedChannels) {
+        QCOMPARE(process.readAllStandardOutput(), QByteArray("line3\n"));
+        if (channelMode2 == QProcess::SeparateChannels)
+            QCOMPARE(process.readAllStandardError(), QByteArray("line3\n"));
+    }
+}
+
 #endif //QT_NO_PROCESS
 
 QTEST_MAIN(tst_QProcess)