QProcess/Linux: add a flag to re-enable the vfork()-like semantics

Commit 29b2fe40dc disabled it by reverting
commit d6bf71123d. We now add the promised
flag to opt-in. The flag is added to all Unix systems, but it really
only applies to Linux right now.

No ChangeLog because the whole UnixProcessParameters structure is new
and has its own changelog.

Task-number: QTBUG-104493
Task-number: QTBUG-111243
Task-number: QTBUG-111964
Change-Id: Icfe44ecf285a480fafe4fffd174d4effd3382495
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

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)

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

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;
+    int ffdflags = FFD_CLOEXEC | useForkFlags(unixExtras.get());
-
-    // QTBUG-86285
-#if defined(Q_OS_LINUX) && !QT_CONFIG(forkfd_pidfd)
-    ffdflags |= FFD_USE_FORK;
-#endif
-    if (unixExtras && unixExtras->childProcessModifier)
-        ffdflags |= FFD_USE_FORK;
-
     forkfd = ::vforkfd(ffdflags, &pid, execChild2, &execChild1);
     int lastForkErrno = errno;
 

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