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

QProcess: align treatment of early start errors

Browse Source 
There are a couple of possible runtime errors that could happen before
the state was changed to QProcess::Starting. This aligns the Unix code
with Windows, which has the state transition at the top, and with the
documentation which says we will enter QProcess::Starting state.

Complements commit 956b2495285251e4840ec32885ffa2cfbb7bd79c, repeating
what it did for Unix (removing the overwriting of the error message that
openChannel() sets) on Windows. We also need to ensure cleanup() is
always called.

Pick-to: 6.6
Change-Id: I76ffba14ece04f24b43efffd17aafdd47f908bf1
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
(cherry picked from commit 79e1389fb9ea4e43e61ab35e41ed275cb76eedbc)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Thiago Macieira 2024-01-16 17:21:37 -08:00 committed by Qt Cherry-pick Bot
parent 068d36dbc1
commit d51033b374
3 changed files with 92 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -521,7 +521,6 @@ bool QProcessPrivate::openChannel(Channel &channel)
setErrorAndEmit(QProcess::FailedToStart,
QProcess::tr("Could not open input redirection for reading"));
}
cleanup();
return false;
} else {
Q_ASSERT_X(channel.process, "QProcess::start", "Internal error");
@ -673,6 +672,7 @@ Q_AUTOTEST_EXPORT bool _qprocessUsingVfork() noexcept
void QProcessPrivate::startProcess()
{
Q_Q(QProcess);
q->setProcessState(QProcess::Starting);
#if defined (QPROCESS_DEBUG)
qDebug("QProcessPrivate::startProcess()");
@ -681,6 +681,8 @@ void QProcessPrivate::startProcess()
// Initialize pipes
if (!openChannels()) {
// openChannel sets the error string
Q_ASSERT(!errorString.isEmpty());
cleanup();
return;
}
if (qt_create_pipe(childStartedPipe) != 0) {
@ -707,7 +709,6 @@ void QProcessPrivate::startProcess()
}
// Start the child.
q->setProcessState(QProcess::Starting);
forkfd = childProcess.startChild(&pid);
int lastForkErrno = errno;

5
src/corelib/io/qprocess_win.cpp
View File

@ -265,7 +265,6 @@ bool QProcessPrivate::openChannel(Channel &channel)
setErrorAndEmit(QProcess::FailedToStart,
QProcess::tr("Could not open output redirection for writing"));
}
cleanup();
return false;
}
case Channel::PipeSource: {
@ -523,9 +522,9 @@ void QProcessPrivate::startProcess()
q->setProcessState(QProcess::Starting);
if (!openChannels()) {
QString errorString = QProcess::tr("Process failed to start: %1").arg(qt_error_string());
// openChannel sets the error string
Q_ASSERT(!errorString.isEmpty());
cleanup();
setErrorAndEmit(QProcess::FailedToStart, errorString);
return;
}

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

@ -88,8 +88,11 @@ private slots:
void environmentIsSorted();
void spaceInName();
void setStandardInputFile();
void setStandardInputFileFailure();
void setStandardOutputFile_data();
void setStandardOutputFile();
void setStandardOutputFileFailure_data() { setStandardOutputFile_data(); }
void setStandardOutputFileFailure();
void setStandardOutputFileNullDevice();
void setStandardOutputFileAndWaitForBytesWritten();
void setStandardOutputProcess_data();
@ -168,6 +171,8 @@ protected slots:
void waitForBytesWrittenInABytesWrittenSlotSlot();
private:
QString nonExistentFileName = u"/this/file/cant/exist/hopefully"_s;
qint64 bytesAvailable;
QTemporaryDir m_temporaryDir;
bool haveWorkingVFork = false;
@ -2319,12 +2324,21 @@ void tst_QProcess::setStandardInputFile()
QProcess process;
QFile file(m_temporaryDir.path() + QLatin1String("/data-sif"));
QSignalSpy stateSpy(&process, &QProcess::stateChanged);
QSignalSpy errorOccurredSpy(&process, &QProcess::errorOccurred);
QVERIFY(file.open(QIODevice::WriteOnly));
file.write(data, sizeof data);
file.close();
process.setStandardInputFile(file.fileName());
process.start("testProcessEcho/testProcessEcho");
QVERIFY(process.waitForStarted());
QCOMPARE(errorOccurredSpy.size(), 0);
QCOMPARE(stateSpy.size(), 2);
QCOMPARE(stateSpy[0][0].value<QProcess::ProcessState>(), QProcess::Starting);
QCOMPARE(stateSpy[1][0].value<QProcess::ProcessState>(), QProcess::Running);
stateSpy.clear();
QVERIFY(process.waitForFinished());
QCOMPARE(process.exitStatus(), QProcess::NormalExit);
@ -2341,6 +2355,25 @@ void tst_QProcess::setStandardInputFile()
QCOMPARE(all.size(), 0);
}
void tst_QProcess::setStandardInputFileFailure()
{
QProcess process;
process.setStandardInputFile(nonExistentFileName);
QSignalSpy stateSpy(&process, &QProcess::stateChanged);
QSignalSpy errorOccurredSpy(&process, &QProcess::errorOccurred);
process.start("testProcessEcho/testProcessEcho");
QVERIFY(!process.waitForStarted());
QCOMPARE(errorOccurredSpy.size(), 1);
QCOMPARE(errorOccurredSpy[0][0].value<QProcess::ProcessError>(), QProcess::FailedToStart);
QCOMPARE(stateSpy.size(), 2);
QCOMPARE(stateSpy[0][0].value<QProcess::ProcessState>(), QProcess::Starting);
QCOMPARE(stateSpy[1][0].value<QProcess::ProcessState>(), QProcess::NotRunning);
}
void tst_QProcess::setStandardOutputFile_data()
{
QTest::addColumn<QProcess::ProcessChannel>("channelToTest");
@ -2394,7 +2427,17 @@ void tst_QProcess::setStandardOutputFile()
else
process.setStandardErrorFile(file.fileName(), mode);
QSignalSpy stateSpy(&process, &QProcess::stateChanged);
QSignalSpy errorOccurredSpy(&process, &QProcess::errorOccurred);
process.start("testProcessEcho2/testProcessEcho2");
QVERIFY(process.waitForStarted());
QCOMPARE(errorOccurredSpy.size(), 0);
QCOMPARE(stateSpy.size(), 2);
QCOMPARE(stateSpy[0][0].value<QProcess::ProcessState>(), QProcess::Starting);
QCOMPARE(stateSpy[1][0].value<QProcess::ProcessState>(), QProcess::Running);
stateSpy.clear();
process.write(testdata, sizeof testdata);
QVERIFY(process.waitForFinished());
QCOMPARE(process.exitStatus(), QProcess::NormalExit);
@ -2419,6 +2462,34 @@ void tst_QProcess::setStandardOutputFile()
QCOMPARE(all.size(), expectedsize);
}
void tst_QProcess::setStandardOutputFileFailure()
{
QFETCH(QProcess::ProcessChannel, channelToTest);
QFETCH(QProcess::ProcessChannelMode, channelMode);
QFETCH(bool, append);
QIODevice::OpenMode mode = append ? QIODevice::Append : QIODevice::Truncate;
// run the process
QProcess process;
process.setProcessChannelMode(channelMode);
if (channelToTest == QProcess::StandardOutput)
process.setStandardOutputFile(nonExistentFileName, mode);
else
process.setStandardErrorFile(nonExistentFileName, mode);
QSignalSpy stateSpy(&process, &QProcess::stateChanged);
QSignalSpy errorOccurredSpy(&process, &QProcess::errorOccurred);
process.start("testProcessEcho2/testProcessEcho2");
QVERIFY(!process.waitForStarted());
QCOMPARE(errorOccurredSpy.size(), 1);
QCOMPARE(errorOccurredSpy[0][0].value<QProcess::ProcessError>(), QProcess::FailedToStart);
QCOMPARE(stateSpy.size(), 2);
QCOMPARE(stateSpy[0][0].value<QProcess::ProcessState>(), QProcess::Starting);
QCOMPARE(stateSpy[1][0].value<QProcess::ProcessState>(), QProcess::NotRunning);
}
void tst_QProcess::setStandardOutputFileNullDevice()
{
static const char testdata[] = "Test data.";
@ -2694,13 +2765,21 @@ void tst_QProcess::setWorkingDirectory()
void tst_QProcess::setNonExistentWorkingDirectory()
{
QProcess process;
process.setWorkingDirectory("this/directory/should/not/exist/for/sure");
process.setWorkingDirectory(nonExistentFileName);
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(int(process.error()), int(QProcess::FailedToStart));
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());
@ -2710,7 +2789,9 @@ void tst_QProcess::setNonExistentWorkingDirectory()
void tst_QProcess::detachedSetNonExistentWorkingDirectory()
{
QProcess process;
process.setWorkingDirectory("this/directory/should/not/exist/for/sure");
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).
@ -2722,6 +2803,9 @@ void tst_QProcess::detachedSetNonExistentWorkingDirectory()
QCOMPARE(process.error(), QProcess::FailedToStart);
QVERIFY(process.errorString() != "Unknown error");
QCOMPARE(errorOccurredSpy.size(), 1);
QCOMPARE(process.error(), QProcess::FailedToStart);
#ifdef Q_OS_UNIX
QVERIFY2(process.errorString().startsWith("chdir:"), process.errorString().toLocal8Bit());
#endif