QProcess/Unix: add failChildProcessModifier()

QProcess detects other types of failures from inside the modifier as
successful starts, because the childStartedPipe gets closed without an
error condition getting written. The new method allows a reporting as a
proper failure-to-start.

Added tests for both cases.

[ChangeLog][QtCore][QProcess] Added failChildProcessModifier().

Change-Id: Icfe44ecf285a480fafe4fffd174da2b10306d3c2
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Thiago Macieira 2023-03-18 15:05:17 -07:00
parent abd2ffc149
commit 90bc0ad41f
5 changed files with 217 additions and 68 deletions

View File

@ -1646,8 +1646,10 @@ std::function<void(void)> QProcess::childProcessModifier() const
\snippet code/src_corelib_io_qprocess.cpp 4
If the modifier function needs to exit the process, remember to use
\c{_exit()}, not \c{exit()}.
If the modifier function experiences a failure condition, it can use
failChildProcessModifier() to report the situation to the QProcess caller.
Alternatively, it may use other methods of stopping the process, like
\c{_exit()}, or \c{abort()}.
Certain properties of the child process, such as closing all extraneous
file descriptors or disconnecting from the controlling TTY, can be more
@ -1674,7 +1676,7 @@ std::function<void(void)> QProcess::childProcessModifier() const
only make use of low-level system calls, such as \c{read()},
\c{write()}, \c{setsid()}, \c{nice()}, and similar.
\sa childProcessModifier(), setUnixProcessParameters()
\sa childProcessModifier(), failChildProcessModifier(), setUnixProcessParameters()
*/
void QProcess::setChildProcessModifier(const std::function<void(void)> &modifier)
{
@ -1684,6 +1686,46 @@ void QProcess::setChildProcessModifier(const std::function<void(void)> &modifier
d->unixExtras->childProcessModifier = modifier;
}
/*!
\fn void QProcess::failChildProcessModifier(const char *description, int error) noexcept
\since 6.7
This functions can be used inside the modifier set with
setChildProcessModifier() to indicate an error condition was encountered.
When the modifier calls these functions, QProcess will emit errorOccurred()
with code QProcess::FailedToStart in the parent process. The \a description
can be used to include some information in errorString() to help diagnose
the problem, usually the name of the call that failed, similar to the C
Library function \c{perror()}. Additionally, the \a error parameter can be
an \c{<errno.h>} error code whose text form will also be included.
For example, a child modifier could prepare an extra file descriptor for
the child process this way:
\code
process.setChildProcessModifier([fd, &process]() {
if (dup2(fd, TargetFileDescriptor) < 0)
process.failChildProcessModifier(errno, "aux comm channel");
});
process.start();
\endcode
Where \c{fd} is a file descriptor currently open in the parent process. If
the \c{dup2()} system call resulted in an \c EBADF condition, the process
errorString() could be "Child process modifier reported error: aux comm
channel: Bad file descriptor".
This function does not return to the caller. Using it anywhere except in
the child modifier and with the correct QProcess object is undefined
behavior.
\note The implementation imposes a length limit to the \a description
parameter to about 500 characters. This does not include the text from the
\a error code.
\sa setChildProcessModifier(), setUnixProcessParameters()
*/
/*!
\since 6.6
Returns the \l UnixProcessParameters object describing extra flags and

View File

@ -174,6 +174,7 @@ public:
#if defined(Q_OS_UNIX) || defined(Q_QDOC)
std::function<void(void)> childProcessModifier() const;
void setChildProcessModifier(const std::function<void(void)> &modifier);
Q_NORETURN void failChildProcessModifier(const char *description, int error = 0) noexcept;
enum UnixProcessFlag : quint32 {
ResetSignalHandlers = 0x0001, // like POSIX_SPAWN_SETSIGDEF

View File

@ -304,7 +304,7 @@ public:
void startProcess();
#if defined(Q_OS_UNIX)
void commitChannels() const;
void execChild(int workingDirectory, char **argv, char **envp) const noexcept;
Q_NORETURN void execChild(int workingDirectory, char **argv, char **envp) const noexcept;
#endif
bool processStarted(QString *errorMessage = nullptr);
void processFinished();

View File

@ -184,8 +184,12 @@ struct AutoPipe
struct ChildError
{
int code;
char function[12];
char function[_POSIX_PIPE_BUF - sizeof(code)];
};
static_assert(std::is_trivial_v<ChildError>);
#ifdef PIPE_BUF
static_assert(PIPE_BUF >= sizeof(ChildError)); // PIPE_BUF may be bigger
#endif
// Used for argv and envp arguments to execve()
struct CharPointerList
@ -645,6 +649,42 @@ static constexpr int FakeErrnoForThrow =
#endif
;
static QString startFailureErrorMessage(ChildError &err, [[maybe_unused]] ssize_t bytesRead)
{
// ChildError is less than the POSIX pipe buffer atomic size, so the read
// must not have been truncated
Q_ASSERT(bytesRead == sizeof(err));
qsizetype len = qstrnlen(err.function, sizeof(err.function));
QString complement = QString::fromUtf8(err.function, len);
if (err.code == FakeErrnoForThrow)
return QProcess::tr("Child process modifier threw an exception");
if (err.code == 0)
return QProcess::tr("Child process modifier reported error: %1")
.arg(std::move(complement));
if (err.code < 0)
return QProcess::tr("Child process modifier reported error: %1: %2")
.arg(std::move(complement), qt_error_string(-err.code));
return QProcess::tr("Child process set up failed: %1: %2")
.arg(std::move(complement), qt_error_string(err.code));
}
Q_NORETURN void
failChildProcess(const QProcessPrivate *d, const char *description, int code) noexcept
{
ChildError error = {};
error.code = code;
strncpy(error.function, description, sizeof(error.function));
qt_safe_write(d->childStartedPipe[1], &error, sizeof(error));
_exit(-1);
}
void QProcess::failChildProcessModifier(const char *description, int error) noexcept
{
// We signal user errors with negative errnos
failChildProcess(d_func(), description, -error);
}
// See IMPORTANT notice below
static void applyProcessParameters(const QProcess::UnixProcessParameters &params)
{
@ -704,22 +744,21 @@ static const char *callChildProcessModifier(const QProcessPrivate::UnixExtras *u
return nullptr;
}
// this function doesn't return if the execution succeeds
static const char *doExecChild(char **argv, char **envp, int workingDirFd,
const QProcessPrivate::UnixExtras *unixExtras) noexcept
Q_NORETURN static void
doExecChild(char **argv, char **envp, int workingDirFd, const QProcessPrivate *d) noexcept
{
// enter the working directory
if (workingDirFd != -1 && fchdir(workingDirFd) == -1)
return "fchdir";
failChildProcess(d, "fchdir", errno);
if (unixExtras) {
if (d->unixExtras) {
// FIRST we call the user modifier function, before we dropping
// privileges or closing non-standard file descriptors
if (const char *what = callChildProcessModifier(unixExtras))
return what;
if (const char *what = callChildProcessModifier(d->unixExtras.get()))
failChildProcess(d, what, FakeErrnoForThrow);
// then we apply our other user-provided parameters
applyProcessParameters(unixExtras->processParameters);
applyProcessParameters(d->unixExtras->processParameters);
}
// execute the process
@ -727,10 +766,9 @@ static const char *doExecChild(char **argv, char **envp, int workingDirFd,
qt_safe_execv(argv[0], argv);
else
qt_safe_execve(argv[0], argv, envp);
return "execve";
failChildProcess(d, "execve", errno);
}
// IMPORTANT:
//
// This function is called in a vfork() context on some OSes (notably, Linux
@ -740,22 +778,13 @@ void QProcessPrivate::execChild(int workingDir, char **argv, char **envp) const
{
QtVforkSafe::ignore_sigpipe(); // reset the signal that we ignored
ChildError error = { 0, {} }; // force zeroing of function[8]
// Render channels configuration.
commitChannels();
// make sure this fd is closed if execv() succeeds
qt_safe_close(childStartedPipe[0]);
const char *what = doExecChild(argv, envp, workingDir, unixExtras.get());
strcpy(error.function, what);
// notify failure
// don't use strerror or any other routines that may allocate memory, since
// some buggy libc versions can deadlock on locked mutexes.
error.code = errno;
qt_safe_write(childStartedPipe[1], &error, sizeof(error));
doExecChild(argv, envp, workingDir, this);
}
bool QProcessPrivate::processStarted(QString *errorMessage)
@ -792,12 +821,8 @@ bool QProcessPrivate::processStarted(QString *errorMessage)
}
// did we read an error message?
if (errorMessage) {
if (buf.code == FakeErrnoForThrow)
*errorMessage = QProcess::tr("childProcessModifier() function threw an exception");
else
*errorMessage = QLatin1StringView(buf.function) + ": "_L1 + qt_error_string(buf.code);
}
if (errorMessage)
*errorMessage = startFailureErrorMessage(buf, ret);
return false;
}
@ -1107,15 +1132,8 @@ void QProcessPrivate::waitForDeadChild()
bool QProcessPrivate::startDetached(qint64 *pid)
{
#ifdef PIPE_BUF
static_assert(PIPE_BUF >= sizeof(ChildError));
#else
static_assert(_POSIX_PIPE_BUF >= sizeof(ChildError));
#endif
ChildError childStatus = { 0, {} };
AutoPipe startedPipe, pidPipe;
childStartedPipe[1] = startedPipe[1];
if (!startedPipe || !pidPipe) {
setErrorAndEmit(QProcess::FailedToStart, "pipe: "_L1 + qt_error_string(errno));
return false;
@ -1155,22 +1173,14 @@ bool QProcessPrivate::startDetached(qint64 *pid)
qt_safe_close(startedPipe[0]);
qt_safe_close(pidPipe[0]);
auto reportFailed = [&](const char *function) {
childStatus.code = errno;
strcpy(childStatus.function, function);
qt_safe_write(startedPipe[1], &childStatus, sizeof(childStatus));
::_exit(1);
};
pid_t doubleForkPid = doFork();
if (doubleForkPid == 0) {
// Render channels configuration.
commitChannels();
reportFailed(doExecChild(argv.pointers.get(), envp.pointers.get(), workingDirFd,
unixExtras.get()));
doExecChild(argv.pointers.get(), envp.pointers.get(), workingDirFd, this);
} else if (doubleForkPid == -1) {
reportFailed("fork: ");
failChildProcess(this, "fork", errno);
}
// success
@ -1198,6 +1208,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
// successfully execve()'d the target process. If it returns any positive
// result, it means one of the two children wrote an error result. Negative
// values should not happen.
ChildError childStatus;
ssize_t startResult = qt_safe_read(startedPipe[0], &childStatus, sizeof(childStatus));
// reap the intermediate child
@ -1213,10 +1224,8 @@ bool QProcessPrivate::startDetached(qint64 *pid)
} else if (!success) {
if (pid)
*pid = -1;
QString msg;
if (startResult == sizeof(childStatus))
msg = QLatin1StringView(childStatus.function) + qt_error_string(childStatus.code);
setErrorAndEmit(QProcess::FailedToStart, msg);
setErrorAndEmit(QProcess::FailedToStart,
startFailureErrorMessage(childStatus, startResult));
}
return success;
}

View File

@ -28,6 +28,10 @@
#include <stdlib.h>
#include "crasher.h"
using namespace Qt::StringLiterals;
typedef void (QProcess::*QProcessErrorSignal)(QProcess::ProcessError);
class tst_QProcess : public QObject
@ -114,7 +118,11 @@ private slots:
#if defined(Q_OS_UNIX)
void setChildProcessModifier_data();
void setChildProcessModifier();
void failChildProcessModifier_data() { setChildProcessModifier_data(); }
void failChildProcessModifier();
void throwInChildProcessModifier();
void terminateInChildProcessModifier_data();
void terminateInChildProcessModifier();
void unixProcessParameters_data();
void unixProcessParameters();
void unixProcessParametersAndChildModifier();
@ -1451,6 +1459,27 @@ void tst_QProcess::createProcessArgumentsModifier()
#endif // Q_OS_WIN
#ifdef Q_OS_UNIX
static constexpr int sigs[] = { SIGABRT, SIGILL, SIGSEGV };
struct DisableCrashLogger
{
// disable core dumps too
tst_QProcessCrash::NoCoreDumps disableCoreDumps {};
std::array<struct sigaction, std::size(sigs)> oldhandlers;
DisableCrashLogger()
{
struct sigaction def = {};
def.sa_handler = SIG_DFL;
for (uint i = 0; i < std::size(sigs); ++i)
sigaction(sigs[i], &def, &oldhandlers[i]);
}
~DisableCrashLogger()
{
// restore them
for (uint i = 0; i < std::size(sigs); ++i)
sigaction(sigs[i], &oldhandlers[i], nullptr);
}
};
static constexpr char messageFromChildProcess[] = "Message from the child process";
static_assert(std::char_traits<char>::length(messageFromChildProcess) <= PIPE_BUF);
static void childProcessModifier(int fd)
@ -1496,6 +1525,36 @@ void tst_QProcess::setChildProcessModifier()
qt_safe_close(pipes[0]);
}
void tst_QProcess::failChildProcessModifier()
{
static const char failureMsg[] =
"Some error message from the child process would go here if this were a "
"real application";
static_assert(sizeof(failureMsg) < _POSIX_PIPE_BUF / 2,
"Implementation detail: the length of the message is limited");
QFETCH(bool, detached);
QProcess process;
process.setChildProcessModifier([&process]() {
process.failChildProcessModifier(failureMsg, EPERM);
});
process.setProgram("testProcessNormal/testProcessNormal");
if (detached) {
qint64 pid;
QVERIFY(!process.startDetached(&pid));
QCOMPARE(pid, -1);
} else {
process.start("testProcessNormal/testProcessNormal");
QVERIFY(!process.waitForStarted(5000));
}
QString errMsg = process.errorString();
QVERIFY2(errMsg.startsWith("Child process modifier reported error: "_L1 + failureMsg),
qPrintable(errMsg));
QVERIFY2(errMsg.endsWith(strerror(EPERM)), qPrintable(errMsg));
}
void tst_QProcess::throwInChildProcessModifier()
{
#ifndef __cpp_exceptions
@ -1511,7 +1570,7 @@ void tst_QProcess::throwInChildProcessModifier()
QVERIFY(!process.waitForStarted(5000));
QCOMPARE(process.state(), QProcess::NotRunning);
QCOMPARE(process.error(), QProcess::FailedToStart);
QVERIFY2(process.errorString().contains("childProcessModifier"),
QVERIFY2(process.errorString().contains("Child process modifier threw an exception"),
qPrintable(process.errorString()));
// try again, to ensure QProcess internal state wasn't corrupted
@ -1519,11 +1578,59 @@ void tst_QProcess::throwInChildProcessModifier()
QVERIFY(!process.waitForStarted(5000));
QCOMPARE(process.state(), QProcess::NotRunning);
QCOMPARE(process.error(), QProcess::FailedToStart);
QVERIFY2(process.errorString().contains("childProcessModifier"),
QVERIFY2(process.errorString().contains("Child process modifier threw an exception"),
qPrintable(process.errorString()));
#endif
}
void tst_QProcess::terminateInChildProcessModifier_data()
{
using F = std::function<void(void)>;
QTest::addColumn<F>("function");
QTest::addColumn<QProcess::ExitStatus>("exitStatus");
QTest::addColumn<bool>("stderrIsEmpty");
QTest::newRow("_exit") << F([]() { _exit(0); }) << QProcess::NormalExit << true;
QTest::newRow("abort") << F(std::abort) << QProcess::CrashExit << true;
QTest::newRow("sigkill") << F([]() { raise(SIGKILL); }) << QProcess::CrashExit << true;
QTest::newRow("terminate") << F(std::terminate) << QProcess::CrashExit
<< (std::get_terminate() == std::abort);
QTest::newRow("crash") << F([]() { tst_QProcessCrash::crash(); }) << QProcess::CrashExit << true;
}
void tst_QProcess::terminateInChildProcessModifier()
{
QFETCH(std::function<void(void)>, function);
QFETCH(QProcess::ExitStatus, exitStatus);
QFETCH(bool, stderrIsEmpty);
// temporarily disable QTest's crash logger
DisableCrashLogger disableCrashLogging;
QProcess process;
process.setChildProcessModifier(function);
process.setProgram("testProcessNormal/testProcessNormal");
// temporarily disable QTest's crash logger while starting the child process
{
DisableCrashLogger d;
process.start();
}
QVERIFY2(process.waitForStarted(5000), qPrintable(process.errorString()));
QVERIFY2(process.waitForFinished(5000), qPrintable(process.errorString()));
QCOMPARE(process.exitStatus(), exitStatus);
// some environments print extra stuff to stderr when we crash
#ifndef Q_OS_QNX
if (!QTestPrivate::isRunningArmOnX86()) {
QByteArray standardError = process.readAllStandardError();
QVERIFY2(standardError.isEmpty() == stderrIsEmpty,
"stderr was: " + standardError);
}
#endif
}
void tst_QProcess::unixProcessParameters_data()
{
QTest::addColumn<QProcess::UnixProcessParameters>("params");
@ -1644,16 +1751,12 @@ void tst_QProcess::unixProcessParametersAndChildModifier()
void tst_QProcess::unixProcessParametersOtherFileDescriptors()
{
constexpr int TargetFileDescriptor = 3;
int pipes[2];
int fd1 = open("/dev/null", O_RDONLY);
int devnull = fcntl(fd1, F_DUPFD, 100); // instead of F_DUPFD_CLOEXEC
pipe2(pipes, O_CLOEXEC);
close(fd1);
auto closeFds = qScopeGuard([&] {
close(devnull);
close(pipes[0]);
// we'll close pipe[1] before any QCOMPARE
});
QProcess process;
@ -1662,20 +1765,14 @@ void tst_QProcess::unixProcessParametersOtherFileDescriptors()
| QProcess::UnixProcessFlag::UseVFork;
params.lowestFileDescriptorToClose = 4;
process.setUnixProcessParameters(params);
process.setChildProcessModifier([devnull, pipes]() {
if (dup2(devnull, TargetFileDescriptor) == TargetFileDescriptor)
return;
write(pipes[1], &errno, sizeof(errno));
_exit(255);
process.setChildProcessModifier([devnull, &process]() {
if (dup2(devnull, TargetFileDescriptor) != TargetFileDescriptor)
process.failChildProcessModifier("dup2", errno);
});
process.setProgram("testUnixProcessParameters/testUnixProcessParameters");
process.setArguments({ "file-descriptors2", QString::number(TargetFileDescriptor),
QString::number(devnull) });
process.start();
close(pipes[1]);
if (int duperror; read(pipes[0], &duperror, sizeof(duperror)) == sizeof(duperror))
QFAIL(QByteArray("dup2 failed: ") + strerror(duperror));
QVERIFY2(process.waitForStarted(5000), qPrintable(process.errorString()));
QVERIFY(process.waitForFinished(5000));