Remove QPROCESS_USE_SPAWN and all that it surrounds

The spawn code was only used to make QProcess work on QNX 6.5.0.  Fork
works on QNX 6.6.0.  The QNX spawn implementation has a flaw that causes
a deadlock in certain situations.  When a working directory is specified
for the process, the QNX spawn implementation stops all threads except
the one doing the spawn so that it can temporarily change the process'
working directory.  This can lead to a deadlock if the thread does
anything that conficts with something being done in a stopped thread.
QNX 6.5.0 is no longer supported in Qt 5.6.0 so we can just switch QNX
to the fork implementation and get rid of the spawn implementation.

Made a QNX specific adjustment to the hardExit test.  There's a bug
in the OS that the test can run into because it does something that
normal applications wouldn't.

Task-number: QTBUG-47250
Change-Id: Ib32567d2c15ce651815858000035ac5aa6f35224
Reviewed-by: Dan Cape <dcape@qnx.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>
Reviewed-by: Rafael Roquetto <rafael.roquetto@kdab.com>

5 files changed, 12 insertions, 235 deletions

diff --git a/src/corelib/io/forkfd_qt.cpp b/src/corelib/io/forkfd_qt.cpp
index b226332cc1..2ccc9ba837 100644
--- a/src/corelib/io/forkfd_qt.cpp
+++ b/src/corelib/io/forkfd_qt.cpp
@@ -35,11 +35,7 @@
 #include <QtCore/qatomic.h>
 #include "qprocess_p.h"
 
-#ifdef QPROCESS_USE_SPAWN
-#  define FORKFD_NO_FORKFD
-#else
-#  define FORKFD_NO_SPAWNFD
-#endif
+#define FORKFD_NO_SPAWNFD
 
 #if defined(QT_NO_DEBUG) && !defined(NDEBUG)
 #  define NDEBUG
diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp
index 952116b9db..a86e3cb438 100644
--- a/src/corelib/io/qprocess.cpp
+++ b/src/corelib/io/qprocess.cpp
@@ -1134,17 +1134,6 @@ bool QProcessPrivate::_q_processDied()
     if (crashed) {
         exitStatus = QProcess::CrashExit;
         setErrorAndEmit(QProcess::Crashed);
-    } else {
-#ifdef QPROCESS_USE_SPAWN
-        // if we're using posix_spawn, waitForStarted always succeeds.
-        // POSIX documents that the sub-process launched by posix_spawn will exit with code
-        // 127 if anything prevents the target program from starting.
-        // http://pubs.opengroup.org/onlinepubs/009695399/functions/posix_spawn.html
-        if (exitStatus == QProcess::NormalExit && exitCode == 127) {
-            setError(QProcess::FailedToStart,
-                     QProcess::tr("Process failed to start (spawned process exited with code 127)"));
-        }
-#endif
     }
 
     bool wasRunning = (processState == QProcess::Running);
diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h
index d3f251c399..436869462c 100644
--- a/src/corelib/io/qprocess_p.h
+++ b/src/corelib/io/qprocess_p.h
@@ -62,9 +62,6 @@ typedef HANDLE Q_PIPE;
 #else
 typedef int Q_PIPE;
 #define INVALID_Q_PIPE -1
-#  ifdef Q_OS_QNX
-#    define QPROCESS_USE_SPAWN
-#  endif
 #endif
 
 #ifndef QT_NO_PROCESS
@@ -347,10 +344,8 @@ public:
 
     void start(QIODevice::OpenMode mode);
     void startProcess();
-#if defined(Q_OS_UNIX) && !defined(QPROCESS_USE_SPAWN)
+#if defined(Q_OS_UNIX)
     void execChild(const char *workingDirectory, char **path, char **argv, char **envp);
-#elif defined(QPROCESS_USE_SPAWN)
-    pid_t spawnChild(pid_t *ppid, const char *workingDirectory, char **argv, char **envp);
 #endif
     bool processStarted(QString *errorMessage = Q_NULLPTR);
     void terminateProcess();
diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 8eb5ac9564..18d203404d 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -423,14 +423,8 @@ void QProcessPrivate::startProcess()
     }
 
     // Start the process manager, and fork off the child process.
-#if defined(QPROCESS_USE_SPAWN)
-    pid_t childPid;
-    forkfd = spawnChild(&childPid, workingDirPtr, argv, envp);
-    Q_ASSUME(forkfd != FFD_CHILD_PROCESS);
-#else
     pid_t childPid;
     forkfd = ::forkfd(FFD_CLOEXEC, &childPid);
-#endif
     int lastForkErrno = errno;
     if (forkfd != FFD_CHILD_PROCESS) {
         // Parent process.
@@ -464,12 +458,10 @@ void QProcessPrivate::startProcess()
     }
 
     // Start the child.
-#if !defined(QPROCESS_USE_SPAWN)
     if (forkfd == FFD_CHILD_PROCESS) {
         execChild(workingDirPtr, path, argv, envp);
         ::_exit(-1);
     }
-#endif
 
     pid = Q_PID(childPid);
 
@@ -508,147 +500,6 @@ void QProcessPrivate::startProcess()
     }
 }
 
-#if defined(QPROCESS_USE_SPAWN)
-static int doSpawn(pid_t *ppid, const posix_spawn_file_actions_t *file_actions,
-                   char **argv, char **envp, const char *workingDir, bool spawn_detached)
-{
-    // A multi threaded QNX Process can't fork so we call spawnfd() instead.
-    posix_spawnattr_t attr;
-    posix_spawnattr_init(&attr);
-#  ifdef Q_OS_QNX
-    posix_spawnattr_setxflags(&attr, POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETPGROUP
-                              | (spawn_detached * POSIX_SPAWN_NOZOMBIE));
-#  else
-    posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETPGROUP);
-#  endif
-    posix_spawnattr_setpgroup(&attr, 0);
-
-    sigset_t sigdefault;
-    sigemptyset(&sigdefault);
-    sigaddset(&sigdefault, SIGPIPE); // reset the signal that we ignored
-    posix_spawnattr_setsigdefault(&attr, &sigdefault);
-
-    // enter the working directory
-    const char *oldWorkingDir = 0;
-    char buff[PATH_MAX + 1];
-
-    if (workingDir) {
-#  ifdef Q_OS_QNX
-        //we need to freeze everyone in order to avoid race conditions with //chdir().
-        if (ThreadCtl(_NTO_TCTL_THREADS_HOLD, 0) == -1)
-            qWarning("ThreadCtl(): cannot hold threads: %s", qPrintable(qt_error_string(errno)));
-#  endif
-
-        oldWorkingDir = QT_GETCWD(buff, PATH_MAX + 1);
-        if (QT_CHDIR(workingDir) == -1)
-            qWarning("ThreadCtl(): failed to chdir to %s", workingDir);
-    }
-
-    int fd;
-    if (spawn_detached) {
-        fd = ::posix_spawn(ppid, argv[0], file_actions, &attr, argv, envp);
-        if (fd == -1) {
-            fd = ::posix_spawnp(ppid, argv[0], file_actions, &attr, argv, envp);
-        }
-    } else {
-        // use spawnfd
-        fd = ::spawnfd(FFD_CLOEXEC | FFD_NONBLOCK, ppid, argv[0], file_actions, &attr, argv, envp);
-        if (fd == -1) {
-            fd = ::spawnfd(FFD_CLOEXEC | FFD_NONBLOCK | FFD_SPAWN_SEARCH_PATH, ppid, argv[0], file_actions,
-                           &attr, argv, envp);
-        }
-    }
-
-    if (oldWorkingDir) {
-        if (QT_CHDIR(oldWorkingDir) == -1)
-            qWarning("ThreadCtl(): failed to chdir to %s", oldWorkingDir);
-
-#  ifdef Q_OS_QNX
-        if (ThreadCtl(_NTO_TCTL_THREADS_CONT, 0) == -1)
-            qFatal("ThreadCtl(): cannot resume threads: %s", qPrintable(qt_error_string(errno)));
-#  endif
-    }
-
-    posix_spawnattr_destroy(&attr);
-    return fd;
-}
-
-pid_t QProcessPrivate::spawnChild(pid_t *ppid, const char *workingDir, char **argv, char **envp)
-{
-    // posix_spawn causes all file descriptors with FD_CLOEXEC to be closed automatically;
-    // we only need to add the actions for our own pipes
-    posix_spawn_file_actions_t file_actions;
-    posix_spawn_file_actions_init(&file_actions);
-
-#   ifdef Q_OS_QNX
-    static const bool OS_QNX = true;
-#   else
-    static const bool OS_QNX = false;
-#endif
-
-    int fdmax = -1;
-
-    if (processChannelMode == QProcess::MergedChannels) {
-        // managed stderr == stdout
-        posix_spawn_file_actions_adddup2(&file_actions, stdoutChannel.pipe[1], STDERR_FILENO);
-
-        if (OS_QNX)
-            fdmax = qMax(fdmax, stdoutChannel.pipe[1]);
-    } else if (processChannelMode != QProcess::ForwardedChannels && processChannelMode != QProcess::ForwardedErrorChannel) {
-        // managed stderr
-        posix_spawn_file_actions_adddup2(&file_actions, stderrChannel.pipe[1], STDERR_FILENO);
-
-        if (OS_QNX)
-            fdmax = qMax(fdmax, stderrChannel.pipe[1]);
-        else
-            posix_spawn_file_actions_addclose(&file_actions, stderrChannel.pipe[1]);
-
-    }
-
-    if (processChannelMode != QProcess::ForwardedChannels && processChannelMode != QProcess::ForwardedOutputChannel) {
-        // managed stdout
-        posix_spawn_file_actions_adddup2(&file_actions, stdoutChannel.pipe[1], STDOUT_FILENO);
-
-        if (OS_QNX)
-            fdmax = qMax(fdmax, stdoutChannel.pipe[1]);
-        else
-            posix_spawn_file_actions_addclose(&file_actions, stdoutChannel.pipe[1]);
-
-    }
-
-    if (inputChannelMode == QProcess::ManagedInputChannel) {
-        posix_spawn_file_actions_adddup2(&file_actions, stdinChannel.pipe[0], STDIN_FILENO);
-
-        if (OS_QNX)
-            fdmax = qMax(fdmax, stdinChannel.pipe[0]);
-        else
-            posix_spawn_file_actions_addclose(&file_actions, stdinChannel.pipe[0]);
-    }
-
-    // Workaround: QNX's spawn implementation will actually dup all FD values
-    // LESS than fdmax - regardless of the FD_CLOEEXEC flag. So we need to add
-    // those to the list of files to close, otherwise dup will fail when some
-    // other thread closes the FD.
-    for (int i = 3; i <= fdmax; i++) {
-        if (::fcntl(i, F_GETFD) & FD_CLOEXEC)
-            posix_spawn_file_actions_addclose(&file_actions, i);
-    }
-
-    int retval = doSpawn(ppid, &file_actions, argv, envp, workingDir, false);
-
-    if (retval == -1) {
-        QString error = qt_error_string(errno);
-        qt_safe_write(childStartedPipe[1], error.data(), error.length() * sizeof(QChar));
-        qt_safe_close(childStartedPipe[1]);
-        childStartedPipe[1] = -1;
-    }
-
-    posix_spawn_file_actions_destroy(&file_actions);
-    return retval;
-}
-
-#else
-
 void QProcessPrivate::execChild(const char *workingDir, char **path, char **argv, char **envp)
 {
     ::signal(SIGPIPE, SIG_DFL);         // reset the signal that we ignored
@@ -716,7 +567,6 @@ report_errno:
     qt_safe_close(childStartedPipe[1]);
     childStartedPipe[1] = -1;
 }
-#endif
 
 bool QProcessPrivate::processStarted(QString *errorMessage)
 {
@@ -1100,40 +950,6 @@ bool QProcessPrivate::waitForDeadChild()
     return true;
 }
 
-#if defined(QPROCESS_USE_SPAWN)
-bool QProcessPrivate::startDetached(const QString &program, const QStringList &arguments, const QString &workingDirectory, qint64 *pid)
-{
-    QList<QByteArray> enc_args;
-    enc_args.append(QFile::encodeName(program));
-    for (int i = 0; i < arguments.size(); ++i)
-        enc_args.append(arguments.at(i).toLocal8Bit());
-
-    const int argc = enc_args.size();
-    QScopedArrayPointer<char*> raw_argv(new char*[argc + 1]);
-    for (int i = 0; i < argc; ++i)
-        raw_argv[i] = const_cast<char *>(enc_args.at(i).data());
-    raw_argv[argc] = 0;
-
-    char **envp = 0; // inherit environment
-
-    // Encode the working directory if it's non-empty, otherwise just pass 0.
-    const char *workingDirPtr = 0;
-    QByteArray encodedWorkingDirectory;
-    if (!workingDirectory.isEmpty()) {
-        encodedWorkingDirectory = QFile::encodeName(workingDirectory);
-        workingDirPtr = encodedWorkingDirectory.constData();
-    }
-
-    pid_t childPid;
-    int retval = doSpawn(&childPid, NULL, raw_argv.data(), envp, workingDirPtr, true);
-    if (pid && retval != -1)
-        *pid = childPid;
-
-    return retval != -1;
-}
-
-#else
-
 bool QProcessPrivate::startDetached(const QString &program, const QStringList &arguments, const QString &workingDirectory, qint64 *pid)
 {
     QByteArray encodedWorkingDirectory = QFile::encodeName(workingDirectory);
@@ -1248,7 +1064,6 @@ bool QProcessPrivate::startDetached(const QString &program, const QStringList &a
     qt_safe_close(pidPipe[0]);
     return success;
 }
-#endif
 
 QT_END_NAMESPACE
 
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
index 46bf1a19e8..7e1d5487ba 100644
--- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
+++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
@@ -45,8 +45,6 @@
 #include <QtNetwork/QHostInfo>
 #include <stdlib.h>
 
-# include <private/qprocess_p.h>    // only so we get QPROCESS_USE_SPAWN
-
 typedef void (QProcess::*QProcessFinishedSignal1)(int);
 typedef void (QProcess::*QProcessFinishedSignal2)(int, QProcess::ExitStatus);
 typedef void (QProcess::*QProcessErrorSignal)(QProcess::ProcessError);
@@ -325,9 +323,6 @@ void tst_QProcess::startDetached()
 {
     QVERIFY(QProcess::startDetached("testProcessNormal/testProcessNormal",
                                     QStringList() << "arg1" << "arg2"));
-#ifdef QPROCESS_USE_SPAWN
-    QEXPECT_FAIL("", "QProcess cannot detect failure to start when using posix_spawn()", Continue);
-#endif
     QCOMPARE(QProcess::startDetached("nonexistingexe"), false);
 }
 
@@ -707,9 +702,6 @@ void tst_QProcess::waitForFinished()
     QCOMPARE(output.count("\n"), 10*1024);
 
     process.start("blurdybloop");
-#if defined(QPROCESS_USE_SPAWN) && !defined(Q_OS_QNX)
-    QEXPECT_FAIL("", "QProcess cannot detect failure to start when using posix_spawn()", Abort);
-#endif
     QVERIFY(!process.waitForFinished());
     QCOMPARE(process.error(), QProcess::FailedToStart);
 }
@@ -922,6 +914,16 @@ void tst_QProcess::hardExit()
 #endif
 
     QVERIFY2(proc.waitForStarted(), qPrintable(proc.errorString()));
+
+#if defined(Q_OS_QNX)
+    // QNX may lose the kill if it's delivered while the forked process
+    // is doing the exec that morphs it into testProcessEcho.  It's very
+    // unlikely that a normal application would do such a thing.  Make
+    // sure the test doesn't accidentally try to do it.
+    proc.write("A");
+    QVERIFY(proc.waitForReadyRead(5000));
+#endif
+
     proc.kill();
 
     QVERIFY(proc.waitForFinished(5000));
@@ -1503,11 +1505,6 @@ void tst_QProcess::nativeArguments()
 void tst_QProcess::exitCodeTest()
 {
     for (int i = 0; i < 255; ++i) {
-#ifdef QPROCESS_USE_SPAWN
-        // POSIX reserves exit code 127 when using posix_spawn
-        if (i == 127)
-            continue;
-#endif
         QProcess process;
         process.start("testExitCodes/testExitCodes " + QString::number(i));
         QVERIFY(process.waitForFinished(5000));
@@ -1518,9 +1515,6 @@ void tst_QProcess::exitCodeTest()
 
 void tst_QProcess::failToStart()
 {
-#if defined(QPROCESS_USE_SPAWN) && !defined(Q_OS_QNX)
-    QSKIP("QProcess cannot detect failure to start when using posix_spawn()");
-#endif
     qRegisterMetaType<QProcess::ProcessError>("QProcess::ProcessError");
     qRegisterMetaType<QProcess::ExitStatus>("QProcess::ExitStatus");
     qRegisterMetaType<QProcess::ProcessState>("QProcess::ProcessState");
@@ -1591,9 +1585,6 @@ void tst_QProcess::failToStart()
 
 void tst_QProcess::failToStartWithWait()
 {
-#if defined(QPROCESS_USE_SPAWN) && !defined(Q_OS_QNX)
-    QSKIP("QProcess cannot detect failure to start when using posix_spawn()");
-#endif
     qRegisterMetaType<QProcess::ProcessError>("QProcess::ProcessError");
     qRegisterMetaType<QProcess::ExitStatus>("QProcess::ExitStatus");
 
@@ -1623,9 +1614,6 @@ void tst_QProcess::failToStartWithWait()
 
 void tst_QProcess::failToStartWithEventLoop()
 {
-#if defined(QPROCESS_USE_SPAWN) && !defined(Q_OS_QNX)
-    QSKIP("QProcess cannot detect failure to start when using posix_spawn()");
-#endif
     qRegisterMetaType<QProcess::ProcessError>("QProcess::ProcessError");
     qRegisterMetaType<QProcess::ExitStatus>("QProcess::ExitStatus");
 
@@ -1912,9 +1900,6 @@ void tst_QProcess::waitForReadyReadForNonexistantProcess()
     QVERIFY(!process.waitForReadyRead()); // used to crash
     process.start("doesntexist");
     QVERIFY(!process.waitForReadyRead());
-#if defined(QPROCESS_USE_SPAWN) && !defined(Q_OS_QNX)
-    QEXPECT_FAIL("", "QProcess cannot detect failure to start when using posix_spawn()", Abort);
-#endif
     QCOMPARE(errorSpy.count(), 1);
     QCOMPARE(errorSpy.at(0).at(0).toInt(), 0);
     QCOMPARE(errorSpy2.count(), 1);
@@ -2282,9 +2267,6 @@ void tst_QProcess::setNonExistentWorkingDirectory()
     // while on Unix with fork it's relative to the child's (with posix_spawn, it could be either).
     process.start(QFileInfo("testSetWorkingDirectory/testSetWorkingDirectory").absoluteFilePath());
     QVERIFY(!process.waitForFinished());
-#ifdef QPROCESS_USE_SPAWN
-    QEXPECT_FAIL("", "QProcess cannot detect failure to start when using posix_spawn()", Continue);
-#endif
     QCOMPARE(int(process.error()), int(QProcess::FailedToStart));
 }
 #endif