whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QProcess/Unix: fix improper restoration of signal mask and cancel state 

By just moving the handling of the child process' desired target
directory below the initialization of either the signal mask and PThread
cancel state, without that "return".

Commit 52ed6af527 ("QProcess/Unix: merge
some code from startProcess() and startDetached()") introduced
QChildProcess and merged the functionality of PThreadCancelGuard into
it. But it added that "return;" to the code path failing to opendirfd()
the target directory, meaning that the QChildProcess constructor could
exit without calling disableThreadCancellations(), but the destructor
would still run restoreThreadCancellations() every time the opening
failed. And we have tests for that: setNonExistentWorkingDirectory and
detachedSetNonExistentWorkingDirectory.

For the cancel state, the uninitialized variable we ended up passing to
pthread_setcancelstate() was probably harmless, because the cancellation
state is almost always active and the variable would have been non-zero.
And we don't test pthread cancellation, so we would never notice the
problem.

But commit bd32c7d705 ("QProcess/Unix:
block all Unix signals between vfork() and exec()") introduced a block
of the Unix signals with the same uninitialized variable problem. Unlike
the PThread cancellation state, the original signal mask would usually
be empty, so the "restoration" would actually mask signals we wanted.
And one such important signal is SIGCHLD, used by QProcess/forkfd when
*not* using vfork semantics. This meant that tests that had a child
process modifier (meaning, they wouldn't use vfork semantics) would end
up timing out because we'd never get the SIGCHLD that told us the child
had exited.

Fixes: QTBUG-123083
Pick-to: 6.7 6.7.0
Change-Id: I1362eb554b97dc012d02eab2dbca90b06728460e
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by:  Alexey Edelev <alexey.edelev@qt.io>
bb10
Thiago Macieira 2024-03-11 11:42:20 -07:00
parent b481393941
commit 418dcf88f8
1 changed files with 9 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -256,15 +256,6 @@ struct QChildProcess
: d(d), argv(resolveExecutable(d->program), d->arguments),
envp(d->environmentPrivate())
{
if (!d->workingDirectory.isEmpty()) {
workingDirectory = opendirfd(QFile::encodeName(d->workingDirectory));
if (workingDirectory < 0) {
d->setErrorAndEmit(QProcess::FailedToStart, "chdir: "_L1 + qt_error_string());
d->cleanup();
return;
}
}
// Block Unix signals, to ensure the user's handlers aren't run in the
// child side and do something weird, especially if the handler and the
// user of QProcess are completely different codebases.
@ -276,6 +267,15 @@ struct QChildProcess
// would be bad enough with regular fork(), but it's likely fatal with
// vfork().
disableThreadCancellations();
if (!d->workingDirectory.isEmpty()) {
workingDirectory = opendirfd(QFile::encodeName(d->workingDirectory));
if (workingDirectory < 0) {
d->setErrorAndEmit(QProcess::FailedToStart, "chdir: "_L1 + qt_error_string());
d->cleanup();
}
}
}
~QChildProcess() noexcept(false)
{