Make process reusable when killing the running one

Try to mimic the behavior of QProcess inside process launcher
as much as possible. After calling QtcProcess::kill() we send
StopProcessPacket to the process launcher - when we receive
it there we start reaping of the process itself and report
back the finished packet with a failure.

Remove canceled state from the LanucherHandle, as after
canceling the process we still need to handle the finished
(or others) signals - like in case of QProcess.

Change-Id: Id66b06449de06675a0ab6778e93e9782afea09d4
Reviewed-by: hjk <hjk@qt.io>

src/libs/utils/launchersocket.cpp

@@ -120,8 +120,6 @@ void LauncherHandle::handlePacket(LauncherPacketType type, const QByteArray &pay
 void LauncherHandle::handleErrorPacket(const QByteArray &packetData)
 {
     QMutexLocker locker(&m_mutex);
-    if (!m_canceled)
-        m_processState = QProcess::NotRunning;
     if (m_waitingFor != SignalType::NoSignal) {
         m_waitCondition.wakeOne();
         m_waitingFor = SignalType::NoSignal;
@@ -166,8 +164,6 @@ void LauncherHandle::handleStartedPacket(const QByteArray &packetData)
 {
     QMutexLocker locker(&m_mutex);
     wakeUpIfWaitingFor(SignalType::Started);
-    if (m_canceled)
-        return;
     m_processState = QProcess::Running;
     const auto packet = LauncherPacket::extractPacket<ProcessStartedPacket>(m_token, packetData);
     m_processId = packet.processId;
@@ -182,8 +178,6 @@ void LauncherHandle::handleReadyReadStandardOutput(const QByteArray &packetData)
 {
     QMutexLocker locker(&m_mutex);
     wakeUpIfWaitingFor(SignalType::ReadyRead);
-    if (m_canceled)
-        return;
     const auto packet = LauncherPacket::extractPacket<ReadyReadStandardOutputPacket>(m_token, packetData);
     if (packet.standardChannel.isEmpty())
         return;
@@ -200,8 +194,6 @@ void LauncherHandle::handleReadyReadStandardError(const QByteArray &packetData)
 {
     QMutexLocker locker(&m_mutex);
     wakeUpIfWaitingFor(SignalType::ReadyRead);
-    if (m_canceled)
-        return;
     const auto packet = LauncherPacket::extractPacket<ReadyReadStandardErrorPacket>(m_token, packetData);
     if (packet.standardChannel.isEmpty())
         return;
@@ -218,8 +210,6 @@ void LauncherHandle::handleFinishedPacket(const QByteArray &packetData)
 {
     QMutexLocker locker(&m_mutex);
     wakeUpIfWaitingFor(SignalType::Finished);
-    if (m_canceled)
-        return;
     m_processState = QProcess::NotRunning;
     const auto packet = LauncherPacket::extractPacket<ProcessFinishedPacket>(m_token, packetData);
     m_exitCode = packet.exitCode;
@@ -268,9 +258,6 @@ bool LauncherHandle::doWaitForSignal(int msecs, SignalType newSignal)
 {
     QMutexLocker locker(&m_mutex);
     QTC_ASSERT(m_waitingFor == SignalType::NoSignal, return false);
-    if (m_canceled) // we don't want to wait if we have canceled it before (ASSERT it?)
-        return false;
-
     // It may happen, that after calling start() and before calling waitForStarted() we might have
     // reached the Running (or even Finished) state already. In this case we should have
     // collected Started (or even Finished) signal to be flushed - so we return true
@@ -325,7 +312,6 @@ void LauncherHandle::cancel()
     }
 
     m_processState = QProcess::NotRunning;
-    m_canceled = true;
 }
 
 void LauncherHandle::start(const QString &program, const QStringList &arguments, QIODevice::OpenMode mode)

src/libs/utils/launchersocket.h

@@ -72,7 +72,6 @@ public:
     QProcess::ProcessState state() const
     { QMutexLocker locker(&m_mutex); return m_processState; }
     void cancel();
-    bool isCanceled() const { return m_canceled; }
 
     QByteArray readAllStandardOutput()
     { QMutexLocker locker(&m_mutex); return readAndClear(m_stdout); }
@@ -163,7 +162,6 @@ private:
     SignalType m_waitingFor = SignalType::NoSignal;
 
     QProcess::ProcessState m_processState = QProcess::NotRunning;
-    std::atomic_bool m_canceled = false;
     std::atomic_bool m_failed = false;
     int m_processId = 0;
     int m_exitCode = 0;

src/tools/processlauncher/launchersockethandler.cpp

@@ -48,6 +48,10 @@ public:
 
     void cancel()
     {
+        if (state() == QProcess::NotRunning) {
+            deleteLater();
+            return;
+        }
         switch (m_stopState) {
         case StopState::Inactive:
             m_stopState = StopState::Terminating;
@@ -61,7 +65,7 @@ public:
             break;
         case StopState::Killing:
             m_stopState = StopState::Inactive;
-            emit failedToStop();
+            deleteLater(); // TODO: employ something like Core::Reaper here
             break;
         }
     }
@@ -74,9 +78,6 @@ public:
 
     quintptr token() const { return m_token; }
 
-signals:
-    void failedToStop();
-
 private:
     const quintptr m_token;
     QTimer * const m_stopTimer;
@@ -217,20 +218,6 @@ void LauncherSocketHandler::handleProcessFinished()
     removeProcess(proc->token());
 }
 
-void LauncherSocketHandler::handleStopFailure()
-{
-    // Process did not react to a kill signal. Rare, but not unheard of.
-    // Forget about the associated Process object and report process exit to the client.
-    Process * proc = senderProcess();
-    ProcessFinishedPacket packet(proc->token());
-    packet.error = QProcess::Crashed;
-    packet.exitCode = -1;
-    packet.exitStatus = QProcess::CrashExit;
-    packet.stdErr = proc->readAllStandardError();
-    packet.stdOut = proc->readAllStandardOutput();
-    sendPacket(packet);
-}
-
 void LauncherSocketHandler::handleStartPacket()
 {
     Process *& process = m_processes[m_packetParser.token()];
@@ -262,6 +249,16 @@ void LauncherSocketHandler::handleStopPacket()
         // This can happen if the process finishes on its own at about the same time the client
         // sends the request.
         logDebug("got stop request when process was not running");
+    } else {
+        // We got the client request to stop the starting / running process.
+        // We report process exit to the client.
+        ProcessFinishedPacket packet(process->token());
+        packet.error = QProcess::Crashed;
+        packet.exitCode = -1;
+        packet.exitStatus = QProcess::CrashExit;
+        packet.stdErr = process->readAllStandardError();
+        packet.stdOut = process->readAllStandardOutput();
+        sendPacket(packet);
     }
     removeProcess(process->token());
 }
@@ -296,7 +293,6 @@ Process *LauncherSocketHandler::setupProcess(quintptr token)
             this, &LauncherSocketHandler::handleReadyReadStandardError);
     connect(p, static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished),
             this, &LauncherSocketHandler::handleProcessFinished);
-    connect(p, &Process::failedToStop, this, &LauncherSocketHandler::handleStopFailure);
     return p;
 }
 

src/tools/processlauncher/launchersockethandler.h

@@ -57,7 +57,6 @@ private:
     void handleReadyReadStandardOutput();
     void handleReadyReadStandardError();
     void handleProcessFinished();
-    void handleStopFailure();
 
     void handleStartPacket();
     void handleStopPacket();