From 6599a6773701ae63886e62414dfba6b0b42f60a9 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Wed, 20 Apr 2022 19:30:59 +0200 Subject: [PATCH] ProcessLauncher: Pass start packet into the launcher socket Instead of holding the start packet on caller side when launcher socket isn't ready yet, pass it into the launcher socket and buffer it there until the socket becomes ready. This simplifies starting the process considerably. Get rid of CallerHandle::canWaitFor(), as this is already checked by QtcProcess itself. Get rid of LauncherHandle::m_waitingFor field and LauncherInterface::isReady() method, as both are not used anymore. Change-Id: Ida6f0629170647249e562028c3ea5db1830b8a0d Reviewed-by: Christian Kandeler Reviewed-by: --- src/libs/utils/launcherinterface.cpp | 6 -- src/libs/utils/launcherinterface.h | 1 - src/libs/utils/launchersocket.cpp | 143 ++++++++------------------- src/libs/utils/launchersocket.h | 20 +--- 4 files changed, 43 insertions(+), 127 deletions(-) diff --git a/src/libs/utils/launcherinterface.cpp b/src/libs/utils/launcherinterface.cpp index a550def367c..6420fb34ff3 100644 --- a/src/libs/utils/launcherinterface.cpp +++ b/src/libs/utils/launcherinterface.cpp @@ -227,12 +227,6 @@ bool LauncherInterface::isStarted() return s_started; } -bool LauncherInterface::isReady() -{ - QMutexLocker locker(&s_instanceMutex); - return instance()->m_private->socket()->isReady(); -} - void LauncherInterface::sendData(const QByteArray &data) { QMutexLocker locker(&s_instanceMutex); diff --git a/src/libs/utils/launcherinterface.h b/src/libs/utils/launcherinterface.h index b7c04dcf8d7..b2e8ac67112 100644 --- a/src/libs/utils/launcherinterface.h +++ b/src/libs/utils/launcherinterface.h @@ -52,7 +52,6 @@ private: friend class Utils::Internal::ProcessLauncherImpl; static bool isStarted(); - static bool isReady(); static void sendData(const QByteArray &data); static Utils::Internal::CallerHandle *registerHandle(QObject *parent, quintptr token); static void unregisterHandle(quintptr token); diff --git a/src/libs/utils/launchersocket.cpp b/src/libs/utils/launchersocket.cpp index 4606af1b437..55e2ee3394c 100644 --- a/src/libs/utils/launchersocket.cpp +++ b/src/libs/utils/launchersocket.cpp @@ -234,32 +234,18 @@ QProcess::ProcessState CallerHandle::state() const return m_processState; } -bool CallerHandle::isStartPacketAwaitingAndClear() -{ - QMutexLocker locker(&m_mutex); - const bool startPacketExisted = m_startPacket.get(); - m_startPacket.reset(); - return startPacketExisted; -} - void CallerHandle::sendStopPacket(StopProcessPacket::SignalType signalType) { if (m_processState == QProcess::NotRunning) return; - if (m_processState == QProcess::Running || !isStartPacketAwaitingAndClear()) { - StopProcessPacket packet(m_token); - packet.signalType = signalType; - sendPacket(packet); - return; - } - - m_processState.store(QProcess::NotRunning); - const QString errorString = QCoreApplication::translate("Utils::LauncherHandle", - "Process was canceled before it was started."); - const ProcessResultData result = { 0, QProcess::NormalExit, QProcess::FailedToStart, - errorString }; - emit done(result); + // TODO: In case m_processState == QProcess::Starting and the launcher socket isn't ready yet + // we might want to remove posted start packet and finish the process immediately. + // In addition, we may always try to check if correspodning start packet for the m_token + // is still awaiting and do the same (remove the packet from the stack and finish immediately). + StopProcessPacket packet(m_token); + packet.signalType = signalType; + sendPacket(packet); } void CallerHandle::terminate() @@ -308,43 +294,24 @@ void CallerHandle::start(const QString &program, const QStringList &arguments) m_command = program; m_arguments = arguments; m_processState = QProcess::Starting; - StartProcessPacket *p = new StartProcessPacket(m_token); - p->command = m_command; - p->arguments = m_arguments; - p->env = m_setup->m_environment.toStringList(); - 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; - p->lowPriority = m_setup->m_lowPriority; - p->unixTerminalDisabled = m_setup->m_unixTerminalDisabled; - p->useCtrlCStub = m_setup->m_useCtrlCStub; - m_startPacket.reset(p); - if (LauncherInterface::isReady()) - doStart(); + StartProcessPacket p(m_token); + p.command = m_command; + p.arguments = m_arguments; + p.env = m_setup->m_environment.toStringList(); + 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; + p.lowPriority = m_setup->m_lowPriority; + p.unixTerminalDisabled = m_setup->m_unixTerminalDisabled; + p.useCtrlCStub = m_setup->m_useCtrlCStub; + sendPacket(p); } -// Called from caller's or launcher's thread. -void CallerHandle::startIfNeeded() -{ - QMutexLocker locker(&m_mutex); - if (m_processState == QProcess::Starting) - doStart(); -} - -// Called from caller's or launcher's thread. Call me with mutex locked. -void CallerHandle::doStart() -{ - if (!m_startPacket) - return; - sendPacket(*m_startPacket); - m_startPacket.reset(); -} - -// Called from caller's or launcher's thread. +// Called from caller's thread exclusively. void CallerHandle::sendPacket(const Internal::LauncherPacket &packet) { LauncherInterface::sendData(packet.serialize()); @@ -384,28 +351,10 @@ void CallerHandle::setProcessSetupData(ProcessSetupData *setup) bool CallerHandle::waitForSignal(int msecs, SignalType newSignal) { QTC_ASSERT(isCalledFromCallersThread(), return false); - if (!canWaitFor(newSignal)) - return false; - if (!m_launcherHandle) - return false; + QTC_ASSERT(m_launcherHandle, return false); return m_launcherHandle->waitForSignal(msecs, newSignal); } -bool CallerHandle::canWaitFor(SignalType newSignal) const -{ - QTC_ASSERT(isCalledFromCallersThread(), return false); - switch (newSignal) { - case SignalType::Started: - return m_processState == QProcess::Starting; - case SignalType::ReadyRead: - case SignalType::Done: - return m_processState != QProcess::NotRunning; - default: - break; - } - return false; -} - // Called from caller's or launcher's thread. bool CallerHandle::isCalledFromCallersThread() const { @@ -428,7 +377,7 @@ bool LauncherHandle::waitForSignal(int msecs, CallerHandle::SignalType newSignal while (true) { if (deadline.hasExpired()) break; - if (!doWaitForSignal(deadline, newSignal)) + if (!doWaitForSignal(deadline)) break; // Matching (or Done) signal was flushed if (m_callerHandle->flushFor(newSignal)) @@ -439,11 +388,10 @@ bool LauncherHandle::waitForSignal(int msecs, CallerHandle::SignalType newSignal } // Called from caller's thread exclusively. -bool LauncherHandle::doWaitForSignal(QDeadlineTimer deadline, CallerHandle::SignalType newSignal) +bool LauncherHandle::doWaitForSignal(QDeadlineTimer deadline) { QMutexLocker locker(&m_mutex); QTC_ASSERT(isCalledFromCallersThread(), return false); - QTC_ASSERT(m_waitingFor == CallerHandle::SignalType::NoSignal, return false); // Flush, if we have any stored signals. // This must be called when holding laucher's mutex locked prior to the call to wait, @@ -451,10 +399,7 @@ bool LauncherHandle::doWaitForSignal(QDeadlineTimer deadline, CallerHandle::Sign if (m_callerHandle->shouldFlush()) return true; - m_waitingFor = newSignal; - const bool ret = m_waitCondition.wait(&m_mutex, deadline); - m_waitingFor = CallerHandle::SignalType::NoSignal; - return ret; + return m_waitCondition.wait(&m_mutex, deadline); } // Called from launcher's thread exclusively. Call me with mutex locked. @@ -552,15 +497,6 @@ void LauncherHandle::handleDonePacket(const QByteArray &packetData) flushCaller(); } -void LauncherHandle::handleSocketReady() -{ - QTC_ASSERT(isCalledFromLaunchersThread(), return); - m_socketError = false; - QMutexLocker locker(&m_mutex); - if (m_callerHandle) - m_callerHandle->startIfNeeded(); -} - void LauncherHandle::handleSocketError(const QString &message) { QTC_ASSERT(isCalledFromLaunchersThread(), return); @@ -618,9 +554,6 @@ LauncherSocket::~LauncherSocket() void LauncherSocket::sendData(const QByteArray &data) { - if (!isReady()) - return; - auto storeRequest = [this](const QByteArray &data) { QMutexLocker locker(&m_mutex); @@ -647,8 +580,6 @@ CallerHandle *LauncherSocket::registerHandle(QObject *parent, quintptr token) // Call it after moving LauncherHandle to the launcher's thread. // Since this method is invoked from caller's thread, CallerHandle will live in caller's thread. m_handles.insert(token, launcherHandle); - connect(this, &LauncherSocket::ready, - launcherHandle, &LauncherHandle::handleSocketReady); connect(this, &LauncherSocket::errorOccurred, launcherHandle, &LauncherHandle::handleSocketError); @@ -685,14 +616,13 @@ void LauncherSocket::setSocket(QLocalSocket *socket) QTC_ASSERT(!m_socket, return); m_socket.store(socket); m_packetParser.setDevice(m_socket); - connect(m_socket, - &QLocalSocket::errorOccurred, + connect(m_socket, &QLocalSocket::errorOccurred, this, &LauncherSocket::handleSocketError); connect(m_socket, &QLocalSocket::readyRead, this, &LauncherSocket::handleSocketDataAvailable); connect(m_socket, &QLocalSocket::disconnected, this, &LauncherSocket::handleSocketDisconnected); - emit ready(); + handleRequests(); } void LauncherSocket::shutdown() @@ -769,11 +699,18 @@ void LauncherSocket::handleRequests() { QTC_ASSERT(isCalledFromLaunchersThread(), return); const auto socket = m_socket.load(); - QTC_ASSERT(socket, return); - QMutexLocker locker(&m_mutex); - for (const QByteArray &request : qAsConst(m_requests)) + if (!socket) + return; + + std::vector requests; + { + QMutexLocker locker(&m_mutex); + requests = m_requests; + m_requests.clear(); + } + + for (const QByteArray &request : qAsConst(requests)) socket->write(request); - m_requests.clear(); } bool LauncherSocket::isCalledFromLaunchersThread() const diff --git a/src/libs/utils/launchersocket.h b/src/libs/utils/launchersocket.h index 6d89a4e100d..dc49157d713 100644 --- a/src/libs/utils/launchersocket.h +++ b/src/libs/utils/launchersocket.h @@ -88,7 +88,6 @@ public: // Called from caller's or launcher's thread. QProcess::ProcessState state() const; - bool isStartPacketAwaitingAndClear(); void sendStopPacket(StopProcessPacket::SignalType signalType); void terminate(); void kill(); @@ -96,8 +95,6 @@ public: qint64 processId() const; void start(const QString &program, const QStringList &arguments); - // Called from caller's or launcher's thread. - void startIfNeeded(); qint64 write(const QByteArray &data); @@ -114,11 +111,8 @@ signals: private: bool waitForSignal(int msecs, SignalType newSignal); - bool canWaitFor(SignalType newSignal) const; - // Called from caller's or launcher's thread. Call me with mutex locked. - void doStart(); - // Called from caller's or launcher's thread. + // Called from caller's thread exclusively. void sendPacket(const Internal::LauncherPacket &packet); // Called from caller's or launcher's thread. bool isCalledFromCallersThread() const; @@ -147,7 +141,6 @@ private: // Modified from caller's thread, read from launcher's thread std::atomic m_processState = QProcess::NotRunning; - std::unique_ptr m_startPacket; int m_processId = 0; QString m_command; @@ -171,7 +164,6 @@ public: void setCallerHandle(CallerHandle *handle) { QMutexLocker locker(&m_mutex); m_callerHandle = handle; } // Called from launcher's thread exclusively. - void handleSocketReady(); void handleSocketError(const QString &message); void handlePacket(LauncherPacketType type, const QByteArray &payload); @@ -180,7 +172,7 @@ public: private: // Called from caller's thread exclusively. - bool doWaitForSignal(QDeadlineTimer deadline, CallerHandle::SignalType newSignal); + bool doWaitForSignal(QDeadlineTimer deadline); // Called from launcher's thread exclusively. Call me with mutex locked. void flushCaller(); // Called from launcher's thread exclusively. @@ -200,8 +192,6 @@ private: QWaitCondition m_waitCondition; const quintptr m_token; std::atomic_bool m_socketError = false; - // Modified only in caller's thread. - CallerHandle::SignalType m_waitingFor = CallerHandle::SignalType::NoSignal; }; class LauncherSocket : public QObject @@ -209,16 +199,12 @@ class LauncherSocket : public QObject Q_OBJECT friend class LauncherInterfacePrivate; public: - // Called from caller's or launcher's thread. - bool isReady() const { return m_socket.load(); } - void sendData(const QByteArray &data); - // Called from caller's thread exclusively. + void sendData(const QByteArray &data); CallerHandle *registerHandle(QObject *parent, quintptr token); void unregisterHandle(quintptr token); signals: - void ready(); void errorOccurred(const QString &error); private: