1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove QPROCESS_USE_SPAWN and all that it surrounds

Browse Source 
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>
This commit is contained in:
James McDonnell 2016-03-18 12:53:12 -04:00
parent 663263c124
commit 005a8bfbf0
5 changed files with 12 additions and 235 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/corelib/io/forkfd_qt.cpp
View File

@ -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
#if defined(QT_NO_DEBUG) && !defined(NDEBUG)
# define NDEBUG

11
src/corelib/io/qprocess.cpp
View File

@ -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);

7
src/corelib/io/qprocess_p.h
View File

@ -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();

185
src/corelib/io/qprocess_unix.cpp
View File

@ -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

38
tests/auto/corelib/io/qprocess/tst_qprocess.cpp
View File

@ -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