diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h index c0273aeeaef..dacf41b77aa 100644 --- a/src/corelib/io/qprocess_p.h +++ b/src/corelib/io/qprocess_p.h @@ -303,7 +303,7 @@ public: void startProcess(); #if defined(Q_OS_UNIX) void commitChannels() const; - void execChild(int workingDirectory, char **argv, char **envp) const; + void execChild(int workingDirectory, char **argv, char **envp) const noexcept; #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 5c72a39911a..d16fa7498e2 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -47,8 +47,40 @@ QT_BEGIN_NAMESPACE +// we need an errno number to use to indicate the child process modifier threw, +// something the regular operations shouldn't set. +static constexpr int FakeErrnoForThrow = +#ifdef ECANCELED + ECANCELED +#else + ESHUTDOWN +#endif + ; + using namespace Qt::StringLiterals; +namespace { +struct PThreadCancelGuard +{ +#if defined(PTHREAD_CANCEL_DISABLE) + int oldstate; + PThreadCancelGuard() noexcept(false) + { + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate); + } + ~PThreadCancelGuard() noexcept(false) + { + reenable(); + } + void reenable() noexcept(false) + { + // this doesn't touch errno + pthread_setcancelstate(oldstate, nullptr); + } +#endif +}; +} + #if !defined(Q_OS_DARWIN) QT_BEGIN_INCLUDE_NAMESPACE @@ -472,6 +504,11 @@ void QProcessPrivate::startProcess() const CharPointerList argv(resolveExecutable(program), arguments); const CharPointerList envp(environment.d.constData()); + // Disable PThread cancellation from this point on: we mustn't have it + // enabled when the child starts running nor while our state could get + // corrupted if we abruptly exited this function. + PThreadCancelGuard cancelGuard; + // Start the child. auto execChild1 = [this, workingDirFd, &argv, &envp]() { execChild(workingDirFd, argv.pointers.get(), envp.pointers.get()); @@ -539,12 +576,24 @@ void QProcessPrivate::startProcess() ::fcntl(stderrChannel.pipe[0], F_SETFL, ::fcntl(stderrChannel.pipe[0], F_GETFL) | O_NONBLOCK); } +static bool callChildProcessModifier(const QProcessPrivate::UnixExtras *unixExtras) noexcept +{ + QT_TRY { + if (unixExtras->childProcessModifier) + unixExtras->childProcessModifier(); + } QT_CATCH (...) { + errno = FakeErrnoForThrow; + return false; + } + return true; +} + // 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(int workingDir, char **argv, char **envp) const +void QProcessPrivate::execChild(int workingDir, char **argv, char **envp) const noexcept { ::signal(SIGPIPE, SIG_DFL); // reset the signal that we ignored @@ -564,8 +613,10 @@ void QProcessPrivate::execChild(int workingDir, char **argv, char **envp) const } if (unixExtras) { - if (unixExtras->childProcessModifier) - unixExtras->childProcessModifier(); + if (!callChildProcessModifier(unixExtras.get())) { + std::strcpy(error.function, "throw"); + goto report_errno; + } } // execute the process @@ -965,6 +1016,9 @@ bool QProcessPrivate::startDetached(qint64 *pid) const CharPointerList argv(resolveExecutable(program), arguments); const CharPointerList envp(environment.d.constData()); + // see startProcess() for more information + PThreadCancelGuard cancelGuard; + pid_t childPid = fork(); if (childPid == 0) { ::signal(SIGPIPE, SIG_DFL); // reset the signal that we ignored diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp index 4d54e8db27f..d10c9ea5662 100644 --- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp +++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp @@ -112,6 +112,7 @@ private slots: #endif // Q_OS_WIN #if defined(Q_OS_UNIX) void setChildProcessModifier(); + void throwInChildProcessModifier(); #endif void exitCodeTest(); void systemEnvironment(); @@ -1469,6 +1470,34 @@ void tst_QProcess::setChildProcessModifier() QCOMPARE(process.exitStatus(), QProcess::NormalExit); QCOMPARE(process.exitCode(), 0); } + +void tst_QProcess::throwInChildProcessModifier() +{ +#ifndef __cpp_exceptions + Q_SKIP("Exceptions disabled."); +#else + QProcess process; + process.setChildProcessModifier([]() { + throw 42; + }); + process.setProgram("testProcessNormal/testProcessNormal"); + + process.start(); + QVERIFY(!process.waitForStarted(5000)); + QCOMPARE(process.state(), QProcess::NotRunning); + QCOMPARE(process.error(), QProcess::FailedToStart); + QVERIFY2(process.errorString().contains("throw"), + qPrintable(process.errorString())); + + // try again, to ensure QProcess internal state wasn't corrupted + process.start(); + QVERIFY(!process.waitForStarted(5000)); + QCOMPARE(process.state(), QProcess::NotRunning); + QCOMPARE(process.error(), QProcess::FailedToStart); + QVERIFY2(process.errorString().contains("throw"), + qPrintable(process.errorString())); +#endif +} #endif void tst_QProcess::exitCodeTest()