Revert commit "don't ever force fork() instead of forkfd()"

This reverts commit d6bf71123d3ef073f25610345cb5dc920e4fb783 and the
docs from commit 82b75570f099911076ad0e144927862e8e359fbd
("QProcess/Linux: fix file descriptor leak in case of failed child
start").

Despite the title of the commit being reverted, the actual consequence
is slightly different: we always use the forkfd() function, but we
change whether we force the use of the fork() library function by use of
the FFD_USE_FORK flag.

Commit 97645478de3ceffce11f58eab140c4c775e48be5 (5.15) first added the
FFD_USE_FORK flag with a hack to detect whether the setupChild() virtual
might have been overwritten. A configure-time feature to force the flag
was added in commit 2ed99ff5ca338ac02f71c347b1449d4662e6c221 (6.0).
Before the 6.0 release, commit d6bf71123d3ef073f25610345cb5dc920e4fb783
removed the conditional use of FFD_USE_FORK, with the changelog message
saying "pthread_atfork() callbacks are consistently not invoked".

We've also since added vfork()-like behavior. We tried it for Qt 5.15
and reverted shortly afterwards because we had got the memory semantics
wrong. Commit e1a787a76ed462e4ed49db78a40c6d7e272182d7 (6.5) finally got
it right, for Linux, which revealed another set of problems with
functions used in the child process modifier.

Therefore, we're going to make vfork() and clone() usage opt-in if the
child process modifier is active. This commit is the first part:
disabling their use by default. The flag to opt in will come in Qt 6.6.

[ChangeLog][QtCore][QProcess] Reverted a change from Qt 6.0 that made
the childProcessModifier() callback be run in a child created by means
other than a real fork() library call, a situation in which certain
other library functions would be unusable, unreliable, or cause
deadlocks. A flag to opt in to the solution with better performance will
be added to Qt 6.6.

Task-number: QTBUG-104493
Fixes: QTBUG-111243
Fixes: QTBUG-111964
Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d3e66843e5a29
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Thiago Macieira 2023-03-17 08:27:26 -07:00 committed by Volker Hilsheimer
parent 2b98dd7645
commit 29b2fe40dc
2 changed files with 2 additions and 9 deletions

View File

@ -1588,15 +1588,6 @@ 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 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<void(void)> &modifier)

View File

@ -471,6 +471,8 @@ void QProcessPrivate::startProcess()
#if defined(Q_OS_LINUX) && !QT_CONFIG(forkfd_pidfd)
ffdflags |= FFD_USE_FORK;
#endif
if (unixExtras && unixExtras->childProcessModifier)
ffdflags |= FFD_USE_FORK;
pid_t childPid;
forkfd = ::vforkfd(ffdflags , &childPid, execChild2, &execChild1);