QProcess/Unix: clean up QChildProcess before calling setErrorAndEmit()

QChildProcess' destructor may unwind the stack, due to the call to
pthread_setcancelstate(), which is why it's marked noexcept(false)
explicitly. To avoid a situation where it causes a stack unwinding while
a stack unwinding is already in progress, this commit makes QProcess
re-enable the thread cancellation state (which immediately processes a
pending cancellation) before calling anything else that may allocate
memory (thus throw). It also guards against calling
pthread_setcancelstate() twice.

The successful execution paths remain under disabled cancellation so C
library calls like close(), fcntl(), read(), and waitpid() will not
cause premature unwinding.

Pick-to: 6.8
Change-Id: I009cf64012e167321aa6fffd609c603cf18b50e4
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
This commit is contained in:
Thiago Macieira 2024-08-28 11:01:32 -07:00
parent 34edfe1344
commit 6763b9f7ec

View File

@ -284,9 +284,15 @@ struct QChildProcess
disableThreadCancellations(); disableThreadCancellations();
} }
~QChildProcess() noexcept(false) ~QChildProcess() noexcept(false)
{
cleanup();
}
void cleanup() noexcept(false)
{ {
if (workingDirectory >= 0) if (workingDirectory >= 0)
close(workingDirectory); close(workingDirectory);
workingDirectory = -1;
restoreSignalMask(); restoreSignalMask();
restoreThreadCancellations(); restoreThreadCancellations();
@ -306,6 +312,7 @@ struct QChildProcess
void restoreSignalMask() const noexcept void restoreSignalMask() const noexcept
{ {
// this function may be called more than once
if (isUsingVfork) if (isUsingVfork)
pthread_sigmask(SIG_SETMASK, &oldsigset, nullptr); pthread_sigmask(SIG_SETMASK, &oldsigset, nullptr);
} }
@ -348,9 +355,11 @@ private:
} }
void restoreThreadCancellations() noexcept(false) void restoreThreadCancellations() noexcept(false)
{ {
if (oldstate != PTHREAD_CANCEL_DISABLE) { // ensure we don't call pthread_setcancelstate() again
int oldoldstate = std::exchange(oldstate, PTHREAD_CANCEL_DISABLE);
if (oldoldstate != PTHREAD_CANCEL_DISABLE) {
// this doesn't touch errno // this doesn't touch errno
pthread_setcancelstate(oldstate, nullptr); pthread_setcancelstate(oldoldstate, nullptr);
} }
} }
#else #else
@ -727,6 +736,7 @@ void QProcessPrivate::startProcess()
if (forkfd == -1) { if (forkfd == -1) {
// Cleanup, report error and return // Cleanup, report error and return
childProcess.cleanup();
#if defined (QPROCESS_DEBUG) #if defined (QPROCESS_DEBUG)
qDebug("fork failed: %ls", qUtf16Printable(qt_error_string(lastForkErrno))); qDebug("fork failed: %ls", qUtf16Printable(qt_error_string(lastForkErrno)));
#endif #endif
@ -1337,6 +1347,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
closeChannels(); closeChannels();
if (childPid == -1) { if (childPid == -1) {
childProcess.cleanup();
setErrorAndEmit(QProcess::FailedToStart, "fork: "_L1 + qt_error_string(savedErrno)); setErrorAndEmit(QProcess::FailedToStart, "fork: "_L1 + qt_error_string(savedErrno));
return false; return false;
} }
@ -1367,6 +1378,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
} else if (!success) { } else if (!success) {
if (pid) if (pid)
*pid = -1; *pid = -1;
childProcess.cleanup();
setErrorAndEmit(QProcess::FailedToStart, setErrorAndEmit(QProcess::FailedToStart,
startFailureErrorMessage(childStatus, startResult)); startFailureErrorMessage(childStatus, startResult));
} }