diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp index 49ba04068a7..16447033ca1 100644 --- a/src/corelib/io/qprocess.cpp +++ b/src/corelib/io/qprocess.cpp @@ -1574,7 +1574,7 @@ std::function QProcess::childProcessModifier() const Sets the \a modifier function for the child process, for Unix systems (including \macos; for Windows, see setCreateProcessArgumentsModifier()). The function contained by the \a modifier argument will be invoked in the - child process after \c{fork()} is completed and QProcess has set up the + child process after \c{fork()} or \c{vfork()} is completed and QProcess has set up the standard file descriptors for the child process, but before \c{execve()}, inside start(). The modifier is useful to change certain properties of the child process, such as setting up additional file descriptors or closing @@ -1595,6 +1595,15 @@ std::function 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 On some systems (notably, Linux), QProcess will 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() */ void QProcess::setChildProcessModifier(const std::function &modifier) diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h index 1331aef1693..929c06480b5 100644 --- a/src/corelib/io/qprocess_p.h +++ b/src/corelib/io/qprocess_p.h @@ -279,9 +279,6 @@ public: bool openChannels(); bool openChannelsForDetached(); bool openChannel(Channel &channel); -#if defined(Q_OS_UNIX) - void commitChannels(); -#endif void closeChannel(Channel *channel); void closeWriteChannel(); void closeChannels(); @@ -308,7 +305,8 @@ public: void start(QIODevice::OpenMode mode); void startProcess(); #if defined(Q_OS_UNIX) - void execChild(const char *workingDirectory, char **argv, char **envp); + void commitChannels() const; + void execChild(const char *workingDirectory, char **argv, char **envp) const; #endif bool processStarted(QString *errorMessage = nullptr); void processFinished(); diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index 449fd14c5c2..7607901021a 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -358,7 +358,7 @@ bool QProcessPrivate::openChannel(Channel &channel) } } -void QProcessPrivate::commitChannels() +void QProcessPrivate::commitChannels() const { // copy the stdin socket if asked to (without closing on exec) if (stdinChannel.pipe[0] != INVALID_Q_PIPE) @@ -516,7 +516,12 @@ void QProcessPrivate::startProcess() ::fcntl(stderrChannel.pipe[0], F_SETFL, ::fcntl(stderrChannel.pipe[0], F_GETFL) | O_NONBLOCK); } -void QProcessPrivate::execChild(const char *workingDir, char **argv, char **envp) +// IMPORTANT: +// +// This function is called in a vfork() context on some OSes (notably, Linux +// with forkfd), so it MUST NOT modify any non-local variable because it's +// still sharing memory with the parent process. +void QProcessPrivate::execChild(const char *workingDir, char **argv, char **envp) const { ::signal(SIGPIPE, SIG_DFL); // reset the signal that we ignored @@ -556,7 +561,6 @@ void QProcessPrivate::execChild(const char *workingDir, char **argv, char **envp report_errno: error.code = errno; qt_safe_write(childStartedPipe[1], &error, sizeof(error)); - childStartedPipe[1] = -1; } bool QProcessPrivate::processStarted(QString *errorMessage)