QProcess::startDetached: Fix behavior change on Windows

Do not overwrite stdout/stderr by default, but only if requested. This restores the behavior of QProcess::startDetached of Qt 5.9. Task-number: QTBUG-67905 Change-Id: Idccf7b0da7bd80f88a0624286ddf2851bc974fb1 Reviewed-by: Friedemann Kleint <[email protected]>
dogandog · Aug 3, 2018 · f0ff73f · f0ff73f
1 parent 8c4207d
commit f0ff73f
Show file tree

Hide file tree

Showing 7 changed files with 139 additions and 30 deletions.
diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp
@@ -2147,6 +2147,10 @@ void QProcess::start(OpenMode mode)
     \endlist
     All other properties of the QProcess object are ignored.
 
+    \note The called process inherits the console window of the calling
+    process. To suppress console output, redirect standard/error output to
+    QProcess::nullDevice().
+
     \sa start()
     \sa startDetached(const QString &program, const QStringList &arguments,
                       const QString &workingDirectory, qint64 *pid)

diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp
@@ -874,6 +874,11 @@ static bool startDetachedUacPrompt(const QString &programIn, const QStringList &
     return true;
 }
 
+static Q_PIPE pipeOrStdHandle(Q_PIPE pipe, DWORD handleNumber)
+{
+    return pipe != INVALID_Q_PIPE ? pipe : GetStdHandle(handleNumber);
+}
+
 bool QProcessPrivate::startDetached(qint64 *pid)
 {
     static const DWORD errorElevationRequired = 740;
@@ -906,15 +911,14 @@ bool QProcessPrivate::startDetached(qint64 *pid)
                                  0, 0, 0,
                                  STARTF_USESTDHANDLES,
                                  0, 0, 0,
-                                 stdinChannel.pipe[0], stdoutChannel.pipe[1], stderrChannel.pipe[1]
+                                 pipeOrStdHandle(stdinChannel.pipe[0], STD_INPUT_HANDLE),
+                                 pipeOrStdHandle(stdoutChannel.pipe[1], STD_OUTPUT_HANDLE),
+                                 pipeOrStdHandle(stderrChannel.pipe[1], STD_ERROR_HANDLE)
                                };
 
-    const bool inheritHandles = stdinChannel.type == Channel::Redirect
-            || stdoutChannel.type == Channel::Redirect
-            || stderrChannel.type == Channel::Redirect;
     QProcess::CreateProcessArguments cpargs = {
         nullptr, reinterpret_cast<wchar_t *>(const_cast<ushort *>(args.utf16())),
-        nullptr, nullptr, inheritHandles, dwCreationFlags, envPtr,
+        nullptr, nullptr, true, dwCreationFlags, envPtr,
         workingDirectory.isEmpty()
             ? nullptr : reinterpret_cast<const wchar_t *>(workingDirectory.utf16()),
         &startupInfo, &pinfo

diff --git a/tests/auto/corelib/io/qprocess/qprocess.pri b/tests/auto/corelib/io/qprocess/qprocess.pri
@@ -11,6 +11,7 @@ SUBPROGRAMS = \
           testProcessEOF \
           testExitCodes \
           testForwarding \
+          testForwardingHelper \
           testGuiProcess \
           testDetached \
           fileWriterProcess \

diff --git a/tests/auto/corelib/io/qprocess/testForwarding/main.cpp b/tests/auto/corelib/io/qprocess/testForwarding/main.cpp
@@ -27,15 +27,32 @@
 ****************************************************************************/
 
 #include <QtCore/QCoreApplication>
+#include <QtCore/QDeadlineTimer>
 #include <QtCore/QProcess>
+#include <QtCore/QTemporaryFile>
+#include <QtCore/QThread>
 
 #include <stdlib.h>
 
+static bool waitForDoneFileWritten(const QString &filePath, int msecs = 30000)
+{
+    QDeadlineTimer t(msecs);
+    do {
+        QThread::msleep(250);
+        QFile file(filePath);
+        if (!file.open(QIODevice::ReadOnly))
+            continue;
+        if (file.readAll() == "That's all folks!")
+            return true;
+    } while (!t.hasExpired());
+    return false;
+}
+
 int main(int argc, char **argv)
 {
     QCoreApplication app(argc, argv);
 
-    if (argc < 3)
+    if (argc < 4)
         return 13;
 
     QProcess process;
@@ -50,23 +67,37 @@ int main(int argc, char **argv)
     if (process.inputChannelMode() != inmode)
         return 11;
 
-    process.start("testProcessEcho2/testProcessEcho2");
+    if (atoi(argv[3])) {
+        QTemporaryFile doneFile("testForwarding_XXXXXX.txt");
+        if (!doneFile.open())
+            return 12;
+        doneFile.close();
+
+        process.setProgram("testForwardingHelper/testForwardingHelper");
+        process.setArguments(QStringList(doneFile.fileName()));
+        if (!process.startDetached())
+            return 13;
+        if (!waitForDoneFileWritten(doneFile.fileName()))
+            return 14;
+    } else {
+        process.start("testProcessEcho2/testProcessEcho2");
 
-    if (!process.waitForStarted(5000))
-        return 2;
+        if (!process.waitForStarted(5000))
+            return 2;
 
-    if (inmode == QProcess::ManagedInputChannel && process.write("forwarded") != 9)
-        return 3;
+        if (inmode == QProcess::ManagedInputChannel && process.write("forwarded") != 9)
+            return 3;
 
-    process.closeWriteChannel();
-    if (!process.waitForFinished(5000))
-        return 4;
+        process.closeWriteChannel();
+        if (!process.waitForFinished(5000))
+            return 4;
 
-    if ((mode == QProcess::ForwardedOutputChannel || mode == QProcess::ForwardedChannels)
+        if ((mode == QProcess::ForwardedOutputChannel || mode == QProcess::ForwardedChannels)
             && !process.readAllStandardOutput().isEmpty())
-        return 5;
-    if ((mode == QProcess::ForwardedErrorChannel || mode == QProcess::ForwardedChannels)
+            return 5;
+        if ((mode == QProcess::ForwardedErrorChannel || mode == QProcess::ForwardedChannels)
             && !process.readAllStandardError().isEmpty())
-        return 6;
+            return 6;
+    }
     return 0;
 }
diff --git a/tests/auto/corelib/io/qprocess/testForwardingHelper/main.cpp b/tests/auto/corelib/io/qprocess/testForwardingHelper/main.cpp
@@ -0,0 +1,45 @@
+/****************************************************************************
+**
+** Copyright (C) 2018 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** 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 https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <fstream>
+#include <stdio.h>
+
+int main(int argc, char *argv[])
+{
+    if (argc < 2) {
+        puts("Usage: testForwardingHelper <doneFilePath>");
+        return 1;
+    }
+    fputs("out data", stdout);
+    fputs("err data", stderr);
+    fflush(stdout);
+    fflush(stderr);
+    std::ofstream out(argv[1]);
+    out << "That's all folks!";
+    return 0;
+}
diff --git a/tests/auto/corelib/io/qprocess/testForwardingHelper/testForwardingHelper.pro b/tests/auto/corelib/io/qprocess/testForwardingHelper/testForwardingHelper.pro
@@ -0,0 +1,4 @@
+SOURCES = main.cpp
+CONFIG -= qt app_bundle
+CONFIG += console
+DESTDIR = ./
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
@@ -1047,32 +1047,50 @@ void tst_QProcess::mergedChannels()
 
 void tst_QProcess::forwardedChannels_data()
 {
+    QTest::addColumn<bool>("detach");
     QTest::addColumn<int>("mode");
     QTest::addColumn<int>("inmode");
     QTest::addColumn<QByteArray>("outdata");
     QTest::addColumn<QByteArray>("errdata");
 
-    QTest::newRow("separate") << int(QProcess::SeparateChannels) << int(QProcess::ManagedInputChannel)
-                              << QByteArray() << QByteArray();
-    QTest::newRow("forwarded") << int(QProcess::ForwardedChannels) << int(QProcess::ManagedInputChannel)
-                               << QByteArray("forwarded") << QByteArray("forwarded");
-    QTest::newRow("stdout") << int(QProcess::ForwardedOutputChannel) << int(QProcess::ManagedInputChannel)
-                            << QByteArray("forwarded") << QByteArray();
-    QTest::newRow("stderr") << int(QProcess::ForwardedErrorChannel) << int(QProcess::ManagedInputChannel)
-                            << QByteArray() << QByteArray("forwarded");
-    QTest::newRow("fwdinput") << int(QProcess::ForwardedErrorChannel) << int(QProcess::ForwardedInputChannel)
-                            << QByteArray() << QByteArray("input");
+    QTest::newRow("separate")
+            << false
+            << int(QProcess::SeparateChannels) << int(QProcess::ManagedInputChannel)
+            << QByteArray() << QByteArray();
+    QTest::newRow("forwarded")
+            << false
+            << int(QProcess::ForwardedChannels) << int(QProcess::ManagedInputChannel)
+            << QByteArray("forwarded") << QByteArray("forwarded");
+    QTest::newRow("stdout")
+            << false
+            << int(QProcess::ForwardedOutputChannel) << int(QProcess::ManagedInputChannel)
+            << QByteArray("forwarded") << QByteArray();
+    QTest::newRow("stderr")
+            << false
+            << int(QProcess::ForwardedErrorChannel) << int(QProcess::ManagedInputChannel)
+            << QByteArray() << QByteArray("forwarded");
+    QTest::newRow("fwdinput")
+            << false
+            << int(QProcess::ForwardedErrorChannel) << int(QProcess::ForwardedInputChannel)
+            << QByteArray() << QByteArray("input");
+    QTest::newRow("detached-default-forwarding")
+            << true
+            << int(QProcess::SeparateChannels) << int(QProcess::ManagedInputChannel)
+            << QByteArray("out data") << QByteArray("err data");
 }
 
 void tst_QProcess::forwardedChannels()
 {
+    QFETCH(bool, detach);
     QFETCH(int, mode);
     QFETCH(int, inmode);
     QFETCH(QByteArray, outdata);
     QFETCH(QByteArray, errdata);
 
     QProcess process;
-    process.start("testForwarding/testForwarding", QStringList() << QString::number(mode) << QString::number(inmode));
+    process.start("testForwarding/testForwarding",
+                  QStringList() << QString::number(mode) << QString::number(inmode)
+                                << QString::number(bool(detach)));
     QVERIFY(process.waitForStarted(5000));
     QCOMPARE(process.write("input"), 5);
     process.closeWriteChannel();
@@ -1089,7 +1107,9 @@ void tst_QProcess::forwardedChannels()
     case 4: err = "did not finish"; break;
     case 5: err = "unexpected stdout"; break;
     case 6: err = "unexpected stderr"; break;
-    case 13: err = "parameter error"; break;
+    case 12: err = "cannot create temp file"; break;
+    case 13: err = "startDetached failed"; break;
+    case 14: err = "waitForDoneFileWritten timed out"; break;
     default: err = "unknown exit code"; break;
     }
     QVERIFY2(!process.exitCode(), err);