1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QProcess/Unix: don't close the childStartedPipe too soon

Browse Source 
We were accidentally closing it along with all the other file
descriptors if the CloseFileDescriptors flag was active. That meant we
were unable to report start problems back to the parent process.

So instead of closing everything immediately, we simply mark everything
as FD_CLOEXEC so they'll be closed by execve()'s success. Because we're
using CLOSE_RANGE_CLOEXEC now to detect the system call, we don't need
the configure-time test.

[ChangeLog][QtCore][QProcess] Fixed a bug that caused QProcess not to
report start failures if the UnixProcessFlag::CloseFileDescriptors flag
was active.

Pick-to: 6.8
Change-Id: I4d81d763281354e886d9fffd56ef6ab8b6115715
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
(cherry picked from commit 3d007ff2e9caf328f92d65f71a94fd869552b1b6)
This commit is contained in:
Thiago Macieira 2025-01-29 13:54:48 -08:00
parent b2191e4450
commit 01010851cb
3 changed files with 91 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/corelib/configure.cmake
View File

@ -104,18 +104,6 @@ clock_gettime(CLOCK_MONOTONIC, &ts);
}
")
# close_range
qt_config_compile_test(close_range
LABEL "close_range()"
CODE
"#include <unistd.h>
int main()
{
return close_range(3, 1024, 0) != 0;
}
")
# cloexec
qt_config_compile_test(cloexec
LABEL "O_CLOEXEC"
@ -610,11 +598,6 @@ qt_feature("clock-monotonic" PUBLIC
CONDITION QT_FEATURE_clock_gettime AND TEST_clock_monotonic
)
qt_feature_definition("clock-monotonic" "QT_NO_CLOCK_MONOTONIC" NEGATE VALUE "1")
qt_feature("close_range" PRIVATE
LABEL "close_range()"
CONDITION QT_FEATURE_process AND TEST_close_range
AUTODETECT UNIX
)
qt_feature("doubleconversion" PRIVATE
LABEL "DoubleConversion"
)

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

@ -43,10 +43,6 @@
#if __has_include(<paths.h>)
# include <paths.h>
#endif
#if __has_include(<linux/close_range.h>)
// FreeBSD's is in <unistd.h>
# include <linux/close_range.h>
#endif
#if QT_CONFIG(process)
#include <forkfd.h>
@ -846,10 +842,13 @@ static const char *applyProcessParameters(const QProcess::UnixProcessParameters
if (params.flags.testFlag(QProcess::UnixProcessFlag::CloseFileDescriptors)) {
int r = -1;
int fd = qMax(STDERR_FILENO + 1, params.lowestFileDescriptorToClose);
#if QT_CONFIG(close_range)
#ifdef CLOSE_RANGE_CLOEXEC
// Mark the file descriptors for closing upon execve() - we delay
// closing so we don't close the ones QProcess needs for itself.
// On FreeBSD, this probably won't fail.
// On Linux, this will fail with ENOSYS before kernel 5.9.
r = close_range(fd, INT_MAX, 0);
// On Linux, this will fail with ENOSYS before kernel 5.9 and EINVAL
// before 5.11.
r = close_range(fd, INT_MAX, CLOSE_RANGE_CLOEXEC);
#endif
if (r == -1) {
// We *could* read /dev/fd to find out what file descriptors are
@ -860,7 +859,7 @@ static const char *applyProcessParameters(const QProcess::UnixProcessParameters
if (struct rlimit limit; getrlimit(RLIMIT_NOFILE, &limit) == 0)
max_fd = limit.rlim_cur;
for ( ; fd < max_fd; ++fd)
close(fd);
fcntl(fd, F_SETFD, FD_CLOEXEC);
}
}

113
tests/auto/corelib/io/qprocess/tst_qprocess.cpp
View File

@ -55,6 +55,8 @@ private slots:
void startWithOldOpen();
void execute();
void startDetached();
void simpleStartFail_data();
void simpleStartFail();
void crashTest();
void crashTest2();
void echoTest_data();
@ -105,8 +107,8 @@ private slots:
void switchReadChannels();
void discardUnwantedOutput();
void setWorkingDirectory();
void setNonExistentWorkingDirectory_data();
void setNonExistentWorkingDirectory();
void detachedSetNonExistentWorkingDirectory();
void exitStatus_data();
void exitStatus();
@ -370,6 +372,62 @@ void tst_QProcess::startDetached()
QCOMPARE(QProcess::startDetached("nonexistingexe"), false);
}
void tst_QProcess::simpleStartFail_data()
{
QTest::addColumn<bool>("detached");
QTest::addColumn<bool>("unixCloseFileDescriptors");
QTest::addRow("normal") << false << false;
QTest::addRow("detached") << true << false;
#ifdef Q_OS_UNIX
// make sure UnixProcessFlag::CloseFileDescriptors doesn't affect our
// error reporting
QTest::addRow("normal+closefds") << false << true;
QTest::addRow("detached+closefds") << true << true;
#endif
}
void tst_QProcess::simpleStartFail()
{
// for more complex and stressful cases, see the other failToStart* tests
QFETCH(bool, detached);
QProcess process;
#ifdef Q_OS_UNIX
QFETCH(bool, unixCloseFileDescriptors);
if (unixCloseFileDescriptors)
process.setUnixProcessParameters(QProcess::UnixProcessFlag::CloseFileDescriptors);
#endif
QSignalSpy stateSpy(&process, &QProcess::stateChanged);
QSignalSpy errorOccurredSpy(&process, &QProcess::errorOccurred);
process.setProgram(nonExistentFileName);
if (detached) {
qint64 pid = -1;
QVERIFY(!process.startDetached(&pid));
QCOMPARE(pid, -1);
} else {
process.start();
QVERIFY(!process.waitForFinished());
QCOMPARE(stateSpy.size(), 2);
QCOMPARE(stateSpy[0][0].value<QProcess::ProcessState>(), QProcess::Starting);
QCOMPARE(stateSpy[1][0].value<QProcess::ProcessState>(), QProcess::NotRunning);
}
QCOMPARE(errorOccurredSpy.size(), 1);
QCOMPARE(process.error(), QProcess::FailedToStart);
QCOMPARE_NE(process.errorString(), "Unknown error");
#ifdef Q_OS_UNIX
QVERIFY2(process.errorString().contains(": execve: "), process.errorString().toLocal8Bit());
QVERIFY2(process.errorString().contains(qt_error_string(ENOENT)), process.errorString().toLocal8Bit());
#endif
}
void tst_QProcess::readFromProcess()
{
QProcess *process = qobject_cast<QProcess *>(sender());
@ -2839,49 +2897,46 @@ void tst_QProcess::setWorkingDirectory()
QCOMPARE(QDir(m_temporaryDir.path()).canonicalPath(), QDir(workingDir.constData()).canonicalPath());
}
void tst_QProcess::setNonExistentWorkingDirectory_data()
{
simpleStartFail_data();
}
void tst_QProcess::setNonExistentWorkingDirectory()
{
QFETCH(bool, detached);
QProcess process;
process.setWorkingDirectory(nonExistentFileName);
#ifdef Q_OS_UNIX
QFETCH(bool, unixCloseFileDescriptors);
if (unixCloseFileDescriptors)
process.setUnixProcessParameters(QProcess::UnixProcessFlag::CloseFileDescriptors);
#endif
QSignalSpy stateSpy(&process, &QProcess::stateChanged);
QSignalSpy errorOccurredSpy(&process, &QProcess::errorOccurred);
// use absolute path because on Windows, the executable is relative to the parent's CWD
// while on Unix with fork it's relative to the child's (with posix_spawn, it could be either).
process.start(QFileInfo("testSetWorkingDirectory/testSetWorkingDirectory").absoluteFilePath());
QVERIFY(!process.waitForFinished());
QCOMPARE(errorOccurredSpy.size(), 1);
QCOMPARE(process.error(), QProcess::FailedToStart);
QCOMPARE(stateSpy.size(), 2);
QCOMPARE(stateSpy[0][0].value<QProcess::ProcessState>(), QProcess::Starting);
QCOMPARE(stateSpy[1][0].value<QProcess::ProcessState>(), QProcess::NotRunning);
#ifdef Q_OS_UNIX
QVERIFY2(process.errorString().startsWith("chdir:"), process.errorString().toLocal8Bit());
#endif
}
void tst_QProcess::detachedSetNonExistentWorkingDirectory()
{
QProcess process;
process.setWorkingDirectory(nonExistentFileName);
QSignalSpy errorOccurredSpy(&process, &QProcess::errorOccurred);
// use absolute path because on Windows, the executable is relative to the parent's CWD
// while on Unix with fork it's relative to the child's (with posix_spawn, it could be either).
process.setProgram(QFileInfo("testSetWorkingDirectory/testSetWorkingDirectory").absoluteFilePath());
qint64 pid = -1;
QVERIFY(!process.startDetached(&pid));
QCOMPARE(pid, -1);
QCOMPARE(process.error(), QProcess::FailedToStart);
QVERIFY(process.errorString() != "Unknown error");
if (detached) {
qint64 pid = -1;
QVERIFY(!process.startDetached(&pid));
QCOMPARE(pid, -1);
} else {
process.start();
QVERIFY(!process.waitForFinished());
QCOMPARE(stateSpy.size(), 2);
QCOMPARE(stateSpy[0][0].value<QProcess::ProcessState>(), QProcess::Starting);
QCOMPARE(stateSpy[1][0].value<QProcess::ProcessState>(), QProcess::NotRunning);
}
QCOMPARE(errorOccurredSpy.size(), 1);
QCOMPARE(process.error(), QProcess::FailedToStart);
QCOMPARE_NE(process.errorString(), "Unknown error");
#ifdef Q_OS_UNIX
QVERIFY2(process.errorString().startsWith("chdir:"), process.errorString().toLocal8Bit());