From a7e8ddd725e4ca6b94c6ba6de7e5978c88e7e82c Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Wed, 18 Aug 2021 10:45:57 +0200 Subject: [PATCH] Add some more comments Add some developer comments explaining which method is designed to be called from a certain thread. Add also some comments about in which thread certain QObjects live in. Change-Id: I38b10216cc29f8a86fd784e588e913407f0fb776 Reviewed-by: hjk --- src/libs/utils/launcherinterface.cpp | 2 ++ src/libs/utils/launchersocket.cpp | 30 +++++++++++++++----------- src/libs/utils/launchersocket.h | 32 ++++++++++++++++++++-------- src/libs/utils/qtcprocess.cpp | 5 ++--- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/src/libs/utils/launcherinterface.cpp b/src/libs/utils/launcherinterface.cpp index b22da63b355..1c67ff9959e 100644 --- a/src/libs/utils/launcherinterface.cpp +++ b/src/libs/utils/launcherinterface.cpp @@ -210,11 +210,13 @@ LauncherInterface::~LauncherInterface() void LauncherInterface::startLauncher() { + // Call in launcher's thread. QMetaObject::invokeMethod(instance().m_private, &LauncherInterfacePrivate::doStart); } void LauncherInterface::stopLauncher() { + // Call in launcher's thread. QMetaObject::invokeMethod(instance().m_private, &LauncherInterfacePrivate::doStop); } diff --git a/src/libs/utils/launchersocket.cpp b/src/libs/utils/launchersocket.cpp index f8f21b573df..43f53590861 100644 --- a/src/libs/utils/launchersocket.cpp +++ b/src/libs/utils/launchersocket.cpp @@ -39,9 +39,10 @@ class CallerHandle : public QObject { Q_OBJECT public: + // Called from caller's thread exclusively, lives in caller's thread. CallerHandle() : QObject() {} - // Always called in caller's thread. Returns the list of flushed signals. + // Called from caller's thread exclusively. Returns the list of flushed signals. QList flush() { QList oldSignals; @@ -70,17 +71,7 @@ public: } return oldSignals; } - void appendSignal(LauncherHandle::SignalType signalType) - { - if (signalType == LauncherHandle::SignalType::NoSignal) - return; - - QMutexLocker locker(&m_mutex); - if (m_signals.contains(signalType)) - return; - - m_signals.append(signalType); - } + // Called from caller's thread exclusively. bool shouldFlushFor(LauncherHandle::SignalType signalType) { // TODO: Should we always flush when the list isn't empty? @@ -93,7 +84,20 @@ public: return true; return false; } + // Called from launcher's thread exclusively. + void appendSignal(LauncherHandle::SignalType signalType) + { + if (signalType == LauncherHandle::SignalType::NoSignal) + return; + + QMutexLocker locker(&m_mutex); + if (m_signals.contains(signalType)) + return; + + m_signals.append(signalType); + } signals: + // Emitted from caller's thread exclusively. void errorOccurred(); void started(); void readyRead(); @@ -485,7 +489,7 @@ void LauncherSocket::sendData(const QByteArray &data) return m_requests.size() == 1; // Returns true if requests handling should be triggered. }; - if (storeRequest(data)) + if (storeRequest(data)) // Call handleRequests() in launcher's thread. QMetaObject::invokeMethod(this, &LauncherSocket::handleRequests); } diff --git a/src/libs/utils/launchersocket.h b/src/libs/utils/launchersocket.h index f39c1c31c6b..360dda8bede 100644 --- a/src/libs/utils/launchersocket.h +++ b/src/libs/utils/launchersocket.h @@ -55,14 +55,11 @@ class CallerHandle; // It's assumed that this object will be alive at least // as long as the corresponding QtcProcess is alive. -// We should have LauncherSocket::registerHandle() and LauncherSocket::unregisterHandle() -// methods. - class LauncherHandle : public QObject { Q_OBJECT public: - // called from main thread + // All the public methods in this class are called exclusively from the caller's thread. bool waitForStarted(int msecs) { return waitForSignal(msecs, SignalType::Started); } bool waitForReadyRead(int msces) @@ -122,25 +119,30 @@ private: ReadyRead, Finished }; - // called from other thread + + // Called from caller's thread exclusively. bool waitForSignal(int msecs, SignalType newSignal); bool doWaitForSignal(int msecs, SignalType newSignal); bool canWaitFor(SignalType newSignal) const; + // Called from caller's or launcher's thread. void doStart(); + // Called from caller's thread exclusively. void slotErrorOccurred(); void slotStarted(); void slotReadyRead(); void slotFinished(); - // called from this thread + // Called from caller's thread, moved to launcher's thread. LauncherHandle(quintptr token, ProcessMode mode) : m_token(token), m_processMode(mode) {} + + // Called from caller's thread exclusively. void createCallerHandle(); void destroyCallerHandle(); + // Called from launcher's thread exclusively. void flushCaller(); - void handlePacket(LauncherPacketType type, const QByteArray &payload); void handleErrorPacket(const QByteArray &packetData); void handleStartedPacket(const QByteArray &packetData); @@ -148,18 +150,22 @@ private: void handleReadyReadStandardError(const QByteArray &packetData); void handleFinishedPacket(const QByteArray &packetData); + // Called from launcher's thread exclusively. void handleSocketReady(); void handleSocketError(const QString &message); + // Called from launcher's thread exclusively. void wakeUpIfWaitingFor(SignalType newSignal); - QByteArray readAndClear(QByteArray &data) + // Called from caller's thread exclusively. + QByteArray readAndClear(QByteArray &data) const { const QByteArray tmp = data; data.clear(); return tmp; } + // Called from caller's or launcher's thread. void sendPacket(const Internal::LauncherPacket &packet); mutable QMutex m_mutex; @@ -169,7 +175,8 @@ private: SignalType m_waitingFor = SignalType::NoSignal; QProcess::ProcessState m_processState = QProcess::NotRunning; - bool m_awaitingShouldContinue = false; // cancel() sets it to false, modified only in caller's thread + // cancel() sets it to false, modified only in caller's thread. + bool m_awaitingShouldContinue = false; int m_processId = 0; int m_exitCode = 0; QProcess::ExitStatus m_exitStatus = QProcess::ExitStatus::NormalExit; @@ -187,6 +194,7 @@ private: QProcess::ProcessChannelMode m_channelMode = QProcess::SeparateChannels; QString m_standardInputFile; + // Lives in caller's thread. CallerHandle *m_callerHandle = nullptr; bool m_belowNormalPriority = false; @@ -203,9 +211,11 @@ 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. LauncherHandle *registerHandle(quintptr token, ProcessMode mode); void unregisterHandle(quintptr token); @@ -214,13 +224,17 @@ signals: void errorOccurred(const QString &error); private: + // Called from caller's thread, moved to launcher's thread. LauncherSocket(QObject *parent = nullptr); + // Called from launcher's thread exclusively. LauncherHandle *handleForToken(quintptr token) const; + // Called from launcher's thread exclusively. void setSocket(QLocalSocket *socket); void shutdown(); + // Called from launcher's thread exclusively. void handleSocketError(); void handleSocketDataAvailable(); void handleSocketDisconnected(); diff --git a/src/libs/utils/qtcprocess.cpp b/src/libs/utils/qtcprocess.cpp index 2be8037d821..f0018e186d4 100644 --- a/src/libs/utils/qtcprocess.cpp +++ b/src/libs/utils/qtcprocess.cpp @@ -310,8 +310,6 @@ private: typedef void (ProcessLauncherImpl::*PreSignal)(void); void cancel(); - void sendPacket(const Internal::LauncherPacket &packet) - { LauncherInterface::socket()->sendData(packet.serialize()); } void handleSocketError(const QString &message); void handleSocketReady(); @@ -319,7 +317,8 @@ private: quintptr token() const { return m_token; } const uint m_token = 0; - LauncherHandle *m_handle = nullptr; // This object lives in a different thread! + // Lives in launcher's thread. + LauncherHandle *m_handle = nullptr; }; void ProcessLauncherImpl::cancel()