From df0593b5e115b4b5c850fb4fd37e6b7ef679219d Mon Sep 17 00:00:00 2001 From: Aaron Barany Date: Tue, 19 Apr 2022 02:28:30 -0700 Subject: [PATCH] Utils: Preserve output order with merged channels Restored the process channel mode members to ProcessSetupData and StartProcessPacket and forward to the QProcess created in LaunchSocketHandler. LaunchSocketHandler now avoids reading from stderr for merged channels since it's guaranteed to not be available. This avoids asserts in Qt 6.3.0 and warnings in Qt 6.3.1 while preserving the original output order when an application has a mix of stdout and stderr output. Change-Id: I9f4541932cf9d8638b38658a5efea9cb5f1798f3 Reviewed-by: Jarek Kobus --- src/libs/utils/launcherpackets.cpp | 4 ++ src/libs/utils/launcherpackets.h | 1 + src/libs/utils/launchersocket.cpp | 1 + src/libs/utils/processinterface.h | 1 + src/libs/utils/qtcprocess.cpp | 39 ++++++++----------- .../processlauncher/launchersockethandler.cpp | 8 +++- 6 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/libs/utils/launcherpackets.cpp b/src/libs/utils/launcherpackets.cpp index 7f8fd595937..9fd7314f756 100644 --- a/src/libs/utils/launcherpackets.cpp +++ b/src/libs/utils/launcherpackets.cpp @@ -64,6 +64,7 @@ void StartProcessPacket::doSerialize(QDataStream &stream) const << env << int(processMode) << writeData + << int(processChannelMode) << standardInputFile << belowNormalPriority << nativeArguments @@ -75,12 +76,14 @@ void StartProcessPacket::doSerialize(QDataStream &stream) const void StartProcessPacket::doDeserialize(QDataStream &stream) { int processModeInt; + int processChannelModeInt; stream >> command >> arguments >> workingDir >> env >> processModeInt >> writeData + >> processChannelModeInt >> standardInputFile >> belowNormalPriority >> nativeArguments @@ -88,6 +91,7 @@ void StartProcessPacket::doDeserialize(QDataStream &stream) >> unixTerminalDisabled >> useCtrlCStub; processMode = Utils::ProcessMode(processModeInt); + processChannelMode = QProcess::ProcessChannelMode(processChannelModeInt); } diff --git a/src/libs/utils/launcherpackets.h b/src/libs/utils/launcherpackets.h index 86472e168ab..896b5a35c5a 100644 --- a/src/libs/utils/launcherpackets.h +++ b/src/libs/utils/launcherpackets.h @@ -112,6 +112,7 @@ public: QStringList env; ProcessMode processMode = ProcessMode::Reader; QByteArray writeData; + QProcess::ProcessChannelMode processChannelMode = QProcess::SeparateChannels; QString standardInputFile; bool belowNormalPriority = false; QString nativeArguments; diff --git a/src/libs/utils/launchersocket.cpp b/src/libs/utils/launchersocket.cpp index 531a1ab5400..4606af1b437 100644 --- a/src/libs/utils/launchersocket.cpp +++ b/src/libs/utils/launchersocket.cpp @@ -315,6 +315,7 @@ void CallerHandle::start(const QString &program, const QStringList &arguments) p->workingDir = m_setup->m_workingDirectory.path(); p->processMode = m_setup->m_processMode; p->writeData = m_setup->m_writeData; + p->processChannelMode = m_setup->m_processChannelMode; p->standardInputFile = m_setup->m_standardInputFile; p->belowNormalPriority = m_setup->m_belowNormalPriority; p->nativeArguments = m_setup->m_nativeArguments; diff --git a/src/libs/utils/processinterface.h b/src/libs/utils/processinterface.h index dda1f6529f5..83344c131e5 100644 --- a/src/libs/utils/processinterface.h +++ b/src/libs/utils/processinterface.h @@ -47,6 +47,7 @@ public: Environment m_environment; Environment m_remoteEnvironment; QByteArray m_writeData; + QProcess::ProcessChannelMode m_processChannelMode = QProcess::SeparateChannels; QVariantHash m_extraData; QString m_standardInputFile; QString m_nativeArguments; // internal, dependent on specific code path diff --git a/src/libs/utils/qtcprocess.cpp b/src/libs/utils/qtcprocess.cpp index 0249917f153..7efe87ad677 100644 --- a/src/libs/utils/qtcprocess.cpp +++ b/src/libs/utils/qtcprocess.cpp @@ -376,6 +376,7 @@ private: m_process->setProcessEnvironment(m_setup.m_environment.toProcessEnvironment()); m_process->setWorkingDirectory(m_setup.m_workingDirectory.path()); m_process->setStandardInputFile(m_setup.m_standardInputFile); + m_process->setProcessChannelMode(m_setup.m_processChannelMode); if (m_setup.m_lowPriority) m_process->setLowPriority(); if (m_setup.m_unixTerminalDisabled) @@ -561,7 +562,6 @@ public: ProcessResult interpretExitCode(int exitCode); QProcess::ProcessState m_state = QProcess::NotRunning; - QProcess::ProcessChannelMode m_processChannelMode = QProcess::SeparateChannels; qint64 m_processId = 0; qint64 m_applicationMainThreadId = 0; ProcessResultData m_resultData; @@ -1145,7 +1145,7 @@ qint64 QtcProcess::applicationMainThreadId() const void QtcProcess::setProcessChannelMode(QProcess::ProcessChannelMode mode) { QTC_CHECK(state() == QProcess::NotRunning); - d->m_processChannelMode = mode; + d->m_setup.m_processChannelMode = mode; } QProcess::ProcessState QtcProcess::state() const @@ -1567,28 +1567,21 @@ void QtcProcessPrivate::handleReadyRead(const QByteArray &outputData, const QByt m_hangTimerCount = 0; // TODO: store a copy of m_processChannelMode on start()? Currently we assert that state // is NotRunning when setting the process channel mode. - if (m_processChannelMode == QProcess::MergedChannels) { - m_stdOut.append(outputData); - m_stdOut.append(errorData); - if (!outputData.isEmpty() || !errorData.isEmpty()) - emitReadyReadStandardOutput(); + if (m_setup.m_processChannelMode == QProcess::ForwardedOutputChannel + || m_setup.m_processChannelMode == QProcess::ForwardedChannels) { + std::cout << outputData.constData() << std::flush; } else { - if (m_processChannelMode == QProcess::ForwardedOutputChannel - || m_processChannelMode == QProcess::ForwardedChannels) { - std::cout << outputData.constData() << std::flush; - } else { - m_stdOut.append(outputData); - if (!outputData.isEmpty()) - emitReadyReadStandardOutput(); - } - if (m_processChannelMode == QProcess::ForwardedErrorChannel - || m_processChannelMode == QProcess::ForwardedChannels) { - std::cerr << errorData.constData() << std::flush; - } else { - m_stdErr.append(errorData); - if (!errorData.isEmpty()) - emitReadyReadStandardError(); - } + m_stdOut.append(outputData); + if (!outputData.isEmpty()) + emitReadyReadStandardOutput(); + } + if (m_setup.m_processChannelMode == QProcess::ForwardedErrorChannel + || m_setup.m_processChannelMode == QProcess::ForwardedChannels) { + std::cerr << errorData.constData() << std::flush; + } else { + m_stdErr.append(errorData); + if (!errorData.isEmpty()) + emitReadyReadStandardError(); } } diff --git a/src/tools/processlauncher/launchersockethandler.cpp b/src/tools/processlauncher/launchersockethandler.cpp index b64e5ad0554..af9e94d3bf3 100644 --- a/src/tools/processlauncher/launchersockethandler.cpp +++ b/src/tools/processlauncher/launchersockethandler.cpp @@ -172,7 +172,8 @@ void LauncherSocketHandler::handleProcessFinished(Process *process) packet.exitStatus = process->exitStatus(); packet.error = process->error(); packet.errorString = process->errorString(); - packet.stdErr = process->readAllStandardError(); + if (process->processChannelMode() != QProcess::MergedChannels) + packet.stdErr = process->readAllStandardError(); packet.stdOut = process->readAllStandardOutput(); sendPacket(packet); removeProcess(process->token()); @@ -193,6 +194,8 @@ void LauncherSocketHandler::handleStartPacket() process->setEnvironment(packet.env); process->setWorkingDirectory(packet.workingDir); // Forwarding is handled by the LauncherInterface + process->setProcessChannelMode(packet.processChannelMode == QProcess::MergedChannels + ? QProcess::MergedChannels : QProcess::SeparateChannels); process->setStandardInputFile(packet.standardInputFile); ProcessStartHandler *handler = process->processStartHandler(); handler->setProcessMode(packet.processMode); @@ -255,7 +258,8 @@ void LauncherSocketHandler::handleStopPacket() packet.error = QProcess::Crashed; packet.exitCode = -1; packet.exitStatus = QProcess::CrashExit; - packet.stdErr = process->readAllStandardError(); + if (process->processChannelMode() != QProcess::MergedChannels) + packet.stdErr = process->readAllStandardError(); packet.stdOut = process->readAllStandardOutput(); sendPacket(packet); }