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 52ed6af5277100ed5b9a4f4231b94013ce539a2c ("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 bd32c7d7055b436b8c33486a5b5ce1c29db77fd4 ("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>

1 files changed, 9 insertions, 9 deletions

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 1179373cf3..ca65a3c776 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -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)
     {