From 99cf99579ea5d79be7516de33cabf49c9ea80456 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Mon, 30 Aug 2021 18:21:41 +0200 Subject: [PATCH] Make access to LauncherSocket thread safe Instead of returning a pointer to LauncherSocket instance, which might get deleted in meantime of just after, route all calls to the LauncherSocket through the LauncherInterface. Make all calls to LauncherInterface secured by the instance mutex. Change-Id: I751228de5f4263112471098ee08cc73a5245147e Reviewed-by: hjk Reviewed-by: Eike Ziller --- src/libs/utils/launcherinterface.cpp | 42 ++++++++++++++++++++++++---- src/libs/utils/launcherinterface.h | 18 ++++++++++-- src/libs/utils/launchersocket.cpp | 6 ++-- src/libs/utils/qtcprocess.cpp | 4 +-- 4 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/libs/utils/launcherinterface.cpp b/src/libs/utils/launcherinterface.cpp index e30c5f8f49d..82975ae8d15 100644 --- a/src/libs/utils/launcherinterface.cpp +++ b/src/libs/utils/launcherinterface.cpp @@ -184,6 +184,7 @@ void LauncherInterfacePrivate::handleProcessStderr() using namespace Utils::Internal; +static QMutex s_instanceMutex; static LauncherInterface *s_instance = nullptr; LauncherInterface::LauncherInterface() @@ -205,6 +206,7 @@ LauncherInterface::~LauncherInterface() // Called from main thread void LauncherInterface::startLauncher(const QString &pathToLauncher) { + QMutexLocker locker(&s_instanceMutex); QTC_ASSERT(s_instance == nullptr, return); s_instance = new LauncherInterface(); LauncherInterfacePrivate *p = s_instance->m_private; @@ -223,6 +225,7 @@ void LauncherInterface::startLauncher(const QString &pathToLauncher) // Called from main thread void LauncherInterface::stopLauncher() { + QMutexLocker locker(&s_instanceMutex); QTC_ASSERT(s_instance != nullptr, return); LauncherInterfacePrivate *p = s_instance->m_private; // Call in launcher's thread. @@ -231,17 +234,44 @@ void LauncherInterface::stopLauncher() s_instance = nullptr; } -Internal::LauncherSocket *LauncherInterface::socket() -{ - QTC_ASSERT(s_instance != nullptr, return nullptr); - return s_instance->m_private->socket(); -} - bool LauncherInterface::isStarted() { + QMutexLocker locker(&s_instanceMutex); return s_instance != nullptr; } +bool LauncherInterface::isReady() +{ + QMutexLocker locker(&s_instanceMutex); + QTC_ASSERT(s_instance != nullptr, return false); + + return s_instance->m_private->socket()->isReady(); +} + +void LauncherInterface::sendData(const QByteArray &data) +{ + QMutexLocker locker(&s_instanceMutex); + QTC_ASSERT(s_instance != nullptr, return); + + s_instance->m_private->socket()->sendData(data); +} + +Utils::Internal::CallerHandle *LauncherInterface::registerHandle(quintptr token, ProcessMode mode) +{ + QMutexLocker locker(&s_instanceMutex); + QTC_ASSERT(s_instance != nullptr, return nullptr); + + return s_instance->m_private->socket()->registerHandle(token, mode); +} + +void LauncherInterface::unregisterHandle(quintptr token) +{ + QMutexLocker locker(&s_instanceMutex); + QTC_ASSERT(s_instance != nullptr, return); + + s_instance->m_private->socket()->unregisterHandle(token); +} + } // namespace Utils #include "launcherinterface.moc" diff --git a/src/libs/utils/launcherinterface.h b/src/libs/utils/launcherinterface.h index 226792499ce..971525f7506 100644 --- a/src/libs/utils/launcherinterface.h +++ b/src/libs/utils/launcherinterface.h @@ -27,13 +27,17 @@ #include "utils_global.h" +#include "processutils.h" + #include #include namespace Utils { namespace Internal { -class LauncherSocket; +class CallerHandle; +class LauncherHandle; class LauncherInterfacePrivate; +class ProcessLauncherImpl; } class QTCREATOR_UTILS_EXPORT LauncherInterface : public QObject @@ -42,13 +46,21 @@ class QTCREATOR_UTILS_EXPORT LauncherInterface : public QObject public: static void startLauncher(const QString &pathToLauncher = {}); static void stopLauncher(); - static Internal::LauncherSocket *socket(); - static bool isStarted(); signals: void errorOccurred(const QString &error); private: + friend class Utils::Internal::CallerHandle; + friend class Utils::Internal::LauncherHandle; + friend class Utils::Internal::ProcessLauncherImpl; + + static bool isStarted(); + static bool isReady(); + static void sendData(const QByteArray &data); + static Utils::Internal::CallerHandle *registerHandle(quintptr token, ProcessMode mode); + static void unregisterHandle(quintptr token); + LauncherInterface(); ~LauncherInterface() override; diff --git a/src/libs/utils/launchersocket.cpp b/src/libs/utils/launchersocket.cpp index 6a5dd9f99d6..c27346f6582 100644 --- a/src/libs/utils/launchersocket.cpp +++ b/src/libs/utils/launchersocket.cpp @@ -283,7 +283,7 @@ void CallerHandle::cancel() m_errorString = QCoreApplication::translate("Utils::LauncherHandle", "Process canceled before it was started."); m_error = QProcess::FailedToStart; - if (LauncherInterface::socket()->isReady()) // TODO: race condition with m_processState??? + if (LauncherInterface::isReady()) // TODO: race condition with m_processState??? sendPacket(StopProcessPacket(m_token)); else emit errorOccurred(m_error); @@ -367,7 +367,7 @@ void CallerHandle::start(const QString &program, const QStringList &arguments, c p->lowPriority = m_lowPriority; p->unixTerminalDisabled = m_unixTerminalDisabled; m_startPacket.reset(p); - if (LauncherInterface::socket()->isReady()) + if (LauncherInterface::isReady()) doStart(); } @@ -391,7 +391,7 @@ void CallerHandle::doStart() // Called from caller's or launcher's thread. void CallerHandle::sendPacket(const Internal::LauncherPacket &packet) { - LauncherInterface::socket()->sendData(packet.serialize()); + LauncherInterface::sendData(packet.serialize()); } qint64 CallerHandle::write(const QByteArray &data) diff --git a/src/libs/utils/qtcprocess.cpp b/src/libs/utils/qtcprocess.cpp index 8bcbe0fc52b..edcdc6807e2 100644 --- a/src/libs/utils/qtcprocess.cpp +++ b/src/libs/utils/qtcprocess.cpp @@ -251,7 +251,7 @@ public: ProcessLauncherImpl(ProcessMode processMode) : ProcessInterface(processMode), m_token(uniqueToken()) { - m_handle = LauncherInterface::socket()->registerHandle(token(), processMode); + m_handle = LauncherInterface::registerHandle(token(), processMode); connect(m_handle, &CallerHandle::errorOccurred, this, &ProcessInterface::errorOccurred); connect(m_handle, &CallerHandle::started, @@ -266,7 +266,7 @@ public: ~ProcessLauncherImpl() override { cancel(); - LauncherInterface::socket()->unregisterHandle(token()); + LauncherInterface::unregisterHandle(token()); } QByteArray readAllStandardOutput() override { return m_handle->readAllStandardOutput(); }