diff options
| -rw-r--r-- | src/corelib/io/qprocess.cpp | 19 | ||||
| -rw-r--r-- | src/corelib/io/qprocess.h | 1 | ||||
| -rw-r--r-- | src/corelib/io/qprocess_unix.cpp | 30 | ||||
| -rw-r--r-- | tests/auto/corelib/io/qprocess/tst_qprocess.cpp | 18 |
4 files changed, 57 insertions, 11 deletions
diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp index ddcd7a4768..ff33607fa1 100644 --- a/src/corelib/io/qprocess.cpp +++ b/src/corelib/io/qprocess.cpp @@ -853,6 +853,15 @@ void QProcessPrivate::Channel::clear() is useful to ensure any ignored (\c SIG_IGN) signal does not affect the child's behavior. + \value UseVFork Requests that QProcess use \c{vfork(2)} to start the child + process. Use this flag to indicate that the callback function set + with setChildProcessModifier() is safe to execute in the child side of + a \c{vfork(2)}; that is, the callback does not modify any non-local + variables (directly or through any function it calls), nor attempts + to communicate with the parent process. It is implementation-defined + if QProcess will actually use \c{vfork(2)} and if \c{vfork(2)} is + different from standard \c{fork(2)}. + \sa setUnixProcessParameters(), unixProcessParameters() */ @@ -1646,6 +1655,16 @@ std::function<void(void)> QProcess::childProcessModifier() const "async-signal-safe" is advised). Most of the Qt API is unsafe inside this callback, including qDebug(), and may lead to deadlocks. + \note If the UnixProcessParameters::UseVFork flag is set via + setUnixProcessParameters(), QProcess may use \c{vfork()} semantics to + start the child process, so this function must obey even stricter + constraints. First, because it is still sharing memory with the parent + process, it must not write to any non-local variable and must obey proper + ordering semantics when reading from them, to avoid data races. Second, + even more library functions may misbehave; therefore, this function should + only make use of low-level system calls, such as \c{read()}, + \c{write()}, \c{setsid()}, \c{nice()}, and similar. + \sa childProcessModifier(), setUnixProcessParameters() */ void QProcess::setChildProcessModifier(const std::function<void(void)> &modifier) diff --git a/src/corelib/io/qprocess.h b/src/corelib/io/qprocess.h index 710b5e2f78..b8923b032d 100644 --- a/src/corelib/io/qprocess.h +++ b/src/corelib/io/qprocess.h @@ -180,6 +180,7 @@ public: IgnoreSigPipe = 0x0002, // some room if we want to add IgnoreSigHup or so CloseNonStandardFileDescriptors = 0x0010, + UseVFork = 0x0020, // like POSIX_SPAWN_USEVFORK }; Q_DECLARE_FLAGS(UnixProcessFlags, UnixProcessFlag) struct UnixProcessParameters diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index c0b10c130e..0064c4f662 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -455,6 +455,26 @@ static QString resolveExecutable(const QString &program) return program; } +static int useForkFlags(const QProcessPrivate::UnixExtras *unixExtras) +{ +#if defined(Q_OS_LINUX) && !QT_CONFIG(forkfd_pidfd) + // some broken environments are known to have problems with the new Linux + // API, so we have a way for users to opt-out during configure time (see + // QTBUG-86285) + return FFD_USE_FORK; +#endif + + if (!unixExtras || !unixExtras->childProcessModifier) + return 0; // no modifier was supplied + + // if a modifier was supplied, use fork() unless the user opts in to + // vfork() + auto flags = unixExtras->processParameters.flags; + if (flags.testFlag(QProcess::UnixProcessFlag::UseVFork)) + return 0; + return FFD_USE_FORK; +} + void QProcessPrivate::startProcess() { Q_Q(QProcess); @@ -515,15 +535,7 @@ void QProcessPrivate::startProcess() return -1; }; - int ffdflags = FFD_CLOEXEC; - - // QTBUG-86285 -#if defined(Q_OS_LINUX) && !QT_CONFIG(forkfd_pidfd) - ffdflags |= FFD_USE_FORK; -#endif - if (unixExtras && unixExtras->childProcessModifier) - ffdflags |= FFD_USE_FORK; - + int ffdflags = FFD_CLOEXEC | useForkFlags(unixExtras.get()); forkfd = ::vforkfd(ffdflags, &pid, execChild2, &execChild1); int lastForkErrno = errno; diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp index 069440c49b..540df04f73 100644 --- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp +++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp @@ -21,6 +21,7 @@ #include <qplatformdefs.h> #ifdef Q_OS_UNIX # include <private/qcore_unix_p.h> +# include <sys/wait.h> #endif #include <QtTest/private/qemulationdetector_p.h> @@ -157,6 +158,7 @@ protected slots: private: qint64 bytesAvailable; QTemporaryDir m_temporaryDir; + bool haveWorkingVFork = false; }; void tst_QProcess::initTestCase() @@ -168,6 +170,12 @@ void tst_QProcess::initTestCase() // chdir to our testdata path and execute helper apps relative to that. QString testdata_dir = QFileInfo(QFINDTESTDATA("testProcessNormal")).absolutePath(); QVERIFY2(QDir::setCurrent(testdata_dir), qPrintable("Could not chdir to " + testdata_dir)); + +#if defined(Q_OS_LINUX) && QT_CONFIG(forkfd_pidfd) + // see detect_clone_pidfd_support() in forkfd_linux.c for explanation + waitid(/*P_PIDFD*/ idtype_t(3), INT_MAX, NULL, WEXITED|WNOHANG); + haveWorkingVFork = (errno == EBADF); +#endif } void tst_QProcess::cleanupTestCase() @@ -1515,6 +1523,7 @@ void tst_QProcess::unixProcessParametersAndChildModifier() static constexpr char message[] = "Message from the handler function\n"; static_assert(std::char_traits<char>::length(message) <= PIPE_BUF); QProcess process; + QAtomicInt vforkControl; int pipes[2]; QVERIFY2(pipe(pipes) == 0, qPrintable(qt_error_string())); @@ -1523,10 +1532,12 @@ void tst_QProcess::unixProcessParametersAndChildModifier() auto pipeGuard1 = qScopeGuard([=] { close(pipes[1]); }); // verify that our modifier runs before the parameters are applied - process.setChildProcessModifier([=] { + process.setChildProcessModifier([=, &vforkControl] { write(pipes[1], message, strlen(message)); + vforkControl.storeRelaxed(1); }); - auto flags = QProcess::UnixProcessFlag::CloseNonStandardFileDescriptors; + auto flags = QProcess::UnixProcessFlag::CloseNonStandardFileDescriptors | + QProcess::UnixProcessFlag::UseVFork; process.setUnixProcessParameters({ flags }); process.setProgram("testUnixProcessParameters/testUnixProcessParameters"); process.setArguments({ "std-file-descriptors", QString::number(pipes[1]) }); @@ -1542,6 +1553,9 @@ void tst_QProcess::unixProcessParametersAndChildModifier() int r = read(pipes[0], buf, sizeof(buf)); QVERIFY2(r >= 0, qPrintable(qt_error_string())); QCOMPARE(QByteArrayView(buf, r), message); + + if (haveWorkingVFork) + QVERIFY2(vforkControl.loadRelaxed(), "QProcess doesn't appear to have used vfork()"); } #endif |