QtcProcess: Fix behavior of kill()

Make it behave more like QProcess::kill().

Before, when QtcProcess::kill() has been called,
the process launcher was putting the process into the
reaper and notified the QtcProcess that it was already killed,
while in fact it could still be alive for a while since it
was in reaper's hands. The current fix makes the behavior
similar to what QProcess does when calling kill(). So now,
in case of a call to kill() the process isn't put into the reaper yet,
so it has a chance to report back the finished signal when the
process was really stopped.

We still use the old behavior of putting the running process
into the reaper in case of a call to QtcProcess::close() and
when d'tor of QtcProcess was called. We don't report back the
confirmation about putting the process into the reaper, since
close() is always called from ProcessLauncherImpl d'tor, so
there is no one to receive this confirmation anyway.

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

src/libs/utils/launcherpackets.h

@@ -143,8 +143,9 @@ public:
     StopProcessPacket(quintptr token);
 
     enum class SignalType {
-        Kill,
-        Terminate
+        Kill, // Calls QProcess::kill
+        Terminate, // Calls QProcess::terminate
+        Close // Puts the process into the reaper, no confirmation signal is being sent.
     };
 
     SignalType signalType = SignalType::Kill;

src/libs/utils/launchersocket.cpp

@@ -262,6 +262,12 @@ void CallerHandle::kill()
     sendStopPacket(StopProcessPacket::SignalType::Kill);
 }
 
+void CallerHandle::close()
+{
+    QTC_ASSERT(isCalledFromCallersThread(), return);
+    sendStopPacket(StopProcessPacket::SignalType::Close);
+}
+
 qint64 CallerHandle::processId() const
 {
     QTC_ASSERT(isCalledFromCallersThread(), return 0);

src/libs/utils/launchersocket.h

@@ -89,6 +89,7 @@ public:
     void sendStopPacket(StopProcessPacket::SignalType signalType);
     void terminate();
     void kill();
+    void close();
 
     qint64 processId() const;
 

src/libs/utils/qtcprocess.cpp

@@ -437,7 +437,7 @@ public:
     }
     ~ProcessLauncherImpl() final
     {
-        m_handle->kill();
+        m_handle->close();
         LauncherInterface::unregisterHandle(token());
         m_handle = nullptr;
     }

src/tools/processlauncher/launchersockethandler.cpp

@@ -242,28 +242,17 @@ void LauncherSocketHandler::handleStopPacket()
                 m_packetParser.token(),
                 m_packetParser.packetData());
 
-    if (packet.signalType == StopProcessPacket::SignalType::Terminate) {
+    switch (packet.signalType) {
+    case StopProcessPacket::SignalType::Terminate:
         process->terminate();
-        return;
-    }
-
-    if (process->state() == QProcess::NotRunning) {
-        // This shouldn't happen, since as soon as process finishes or error occurrs
-        // the process is being removed.
-        logWarn("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.
-        ProcessDonePacket packet(process->token());
-        packet.error = QProcess::Crashed;
-        packet.exitCode = -1;
-        packet.exitStatus = QProcess::CrashExit;
-        if (process->processChannelMode() != QProcess::MergedChannels)
-            packet.stdErr = process->readAllStandardError();
-        packet.stdOut = process->readAllStandardOutput();
-        sendPacket(packet);
-    }
+        break;
+    case StopProcessPacket::SignalType::Kill:
+        process->kill();
+        break;
+    case StopProcessPacket::SignalType::Close:
         removeProcess(process->token());
+        break;
+    }
 }
 
 void LauncherSocketHandler::handleShutdownPacket()