SshProcessInterface: Remove pidArgumentForKill()

This interface method wasn't really sensible.

Change-Id: Ia47c893886ec06a2263b96d161578d46d2df5ffa
Reviewed-by: hjk <hjk@qt.io>
Reviewed-by: Marcus Tillmanns <marcus.tillmanns@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/libs/utils/processinterface.h

@@ -86,8 +86,6 @@ class QTCREATOR_UTILS_EXPORT ProcessInterface : public QObject
 public:
     ProcessInterface(QObject *parent = nullptr) : QObject(parent) {}
 
-    static int controlSignalToInt(ControlSignal controlSignal);
-
 signals:
     // This should be emitted when being in Starting state only.
     // After emitting this signal the process enters Running state.
@@ -102,6 +100,8 @@ signals:
     void done(const Utils::ProcessResultData &resultData);
 
 protected:
+    static int controlSignalToInt(ControlSignal controlSignal);
+
     ProcessSetupData m_setup;
 
 private:

src/plugins/qnx/qnxdevice.cpp

@@ -57,7 +57,7 @@ public:
 
 private:
     QString fullCommandLine(const CommandLine &commandLine) const final;
-    QString pidArgumentForKill() const final;
+    void sendControlSignal(Utils::ControlSignal controlSignal) final;
 
     const QString m_pidFile;
 };
@@ -95,9 +95,14 @@ QString QnxProcessImpl::fullCommandLine(const CommandLine &commandLine) const
     return fullCommandLine;
 }
 
-QString QnxProcessImpl::pidArgumentForKill() const
+void QnxProcessImpl::sendControlSignal(Utils::ControlSignal controlSignal)
 {
-    return QString::fromLatin1("`cat %1`").arg(m_pidFile);
+    QTC_ASSERT(controlSignal != ControlSignal::KickOff, return);
+    const QString args = QString::fromLatin1("-%1 `cat %2`")
+            .arg(controlSignalToInt(controlSignal)).arg(m_pidFile);
+    CommandLine command = { "kill", args, CommandLine::Raw };
+    // Note: This blocking call takes up to 2 ms for local remote.
+    runInShell(command);
 }
 
 const char QnxVersionKey[] = "QnxVersion";

src/plugins/remotelinux/CMakeLists.txt

@@ -16,6 +16,7 @@ add_qtc_plugin(RemoteLinux
     linuxdevice.cpp linuxdevice.h
     linuxdeviceprocess.cpp linuxdeviceprocess.h
     linuxdevicetester.cpp linuxdevicetester.h
+    linuxprocessinterface.h
     makeinstallstep.cpp makeinstallstep.h
     packageuploader.cpp packageuploader.h
     publickeydeploymentdialog.cpp publickeydeploymentdialog.h

src/plugins/remotelinux/linuxdevice.cpp

@@ -29,6 +29,7 @@
 #include "genericlinuxdeviceconfigurationwizard.h"
 #include "linuxdeviceprocess.h"
 #include "linuxdevicetester.h"
+#include "linuxprocessinterface.h"
 #include "publickeydeploymentdialog.h"
 #include "remotelinux_constants.h"
 #include "remotelinuxsignaloperation.h"
@@ -442,7 +443,6 @@ public:
     SshProcessInterfacePrivate(SshProcessInterface *sshInterface, LinuxDevicePrivate *devicePrivate);
 
     void start();
-    void sendControlSignal(ControlSignal controlSignal);
 
     void handleConnected(const QString &socketFilePath);
     void handleDisconnected(const ProcessResultData &result);
@@ -459,16 +459,17 @@ public:
     SshProcessInterface *q = nullptr;
 
     qint64 m_processId = 0;
-    QtcProcess m_process;
-    LinuxDevicePrivate *m_devicePrivate = nullptr;
     // Store the IDevice::ConstPtr in order to extend the lifetime of device for as long
     // as this object is alive.
     IDevice::ConstPtr m_device;
     std::unique_ptr<SshConnectionHandle> m_connectionHandle;
+    QtcProcess m_process;
+    LinuxDevicePrivate *m_devicePrivate = nullptr;
 
     QString m_socketFilePath;
     SshConnectionParameters m_sshParameters;
     bool m_connecting = false;
+    bool m_killed = false;
 
     ProcessResultData m_result;
 };
@@ -501,8 +502,20 @@ void SshProcessInterface::emitStarted(qint64 processId)
 
 void SshProcessInterface::killIfRunning()
 {
-    if (d->m_process.state() == QProcess::Running)
+    if (d->m_killed || d->m_process.state() != QProcess::Running)
+        return;
     sendControlSignal(ControlSignal::Kill);
+    d->m_killed = true;
+}
+
+qint64 SshProcessInterface::processId() const
+{
+    return d->m_processId;
+}
+
+bool SshProcessInterface::runInShell(const CommandLine &command, const QByteArray &data)
+{
+    return d->m_devicePrivate->runInShell(command, data);
 }
 
 void SshProcessInterface::start()
@@ -515,11 +528,6 @@ qint64 SshProcessInterface::write(const QByteArray &data)
     return d->m_process.writeRaw(data);
 }
 
-void SshProcessInterface::sendControlSignal(ControlSignal controlSignal)
-{
-    d->sendControlSignal(controlSignal);
-}
-
 bool SshProcessInterface::waitForStarted(int msecs)
 {
     Q_UNUSED(msecs)
@@ -541,30 +549,29 @@ bool SshProcessInterface::waitForFinished(int msecs)
     return false;
 }
 
-class LinuxProcessImpl final : public SshProcessInterface
-{
-    Q_OBJECT
-
-public:
-    LinuxProcessImpl(const LinuxDevice *linuxDevice);
-    ~LinuxProcessImpl() { killIfRunning(); }
-
-private:
-    void handleStarted(qint64 processId) final;
-    void handleReadyReadStandardOutput(const QByteArray &outputData) final;
-
-    QString fullCommandLine(const Utils::CommandLine &commandLine) const final;
-
-    QByteArray m_output;
-    bool m_pidParsed = false;
-};
-
-LinuxProcessImpl::LinuxProcessImpl(const LinuxDevice *linuxDevice)
+LinuxProcessInterface::LinuxProcessInterface(const LinuxDevice *linuxDevice)
     : SshProcessInterface(linuxDevice)
 {
 }
 
-QString LinuxProcessImpl::fullCommandLine(const CommandLine &commandLine) const
+LinuxProcessInterface::~LinuxProcessInterface()
+{
+    killIfRunning();
+}
+
+void LinuxProcessInterface::sendControlSignal(ControlSignal controlSignal)
+{
+    QTC_ASSERT(controlSignal != ControlSignal::KickOff, return);
+    const qint64 pid = processId();
+    QTC_ASSERT(pid, return); // TODO: try sending a signal based on process name
+    const QString args = QString::fromLatin1("-%1 -%2 %2")
+            .arg(controlSignalToInt(controlSignal)).arg(pid);
+    CommandLine command = { "kill", args, CommandLine::Raw };
+    // Note: This blocking call takes up to 2 ms for local remote.
+    runInShell(command);
+}
+
+QString LinuxProcessInterface::fullCommandLine(const CommandLine &commandLine) const
 {
     CommandLine cmd;
 
@@ -598,7 +605,7 @@ QString LinuxProcessImpl::fullCommandLine(const CommandLine &commandLine) const
     return cmd.arguments();
 }
 
-void LinuxProcessImpl::handleStarted(qint64 processId)
+void LinuxProcessInterface::handleStarted(qint64 processId)
 {
     // Don't emit started() when terminal is off,
     // it's being done later inside handleReadyReadStandardOutput().
@@ -609,7 +616,7 @@ void LinuxProcessImpl::handleStarted(qint64 processId)
     emitStarted(processId);
 }
 
-void LinuxProcessImpl::handleReadyReadStandardOutput(const QByteArray &outputData)
+void LinuxProcessInterface::handleReadyReadStandardOutput(const QByteArray &outputData)
 {
     if (m_pidParsed) {
         emit readyRead(outputData, {});
@@ -645,9 +652,9 @@ SshProcessInterfacePrivate::SshProcessInterfacePrivate(SshProcessInterface *sshI
                                                        LinuxDevicePrivate *devicePrivate)
     : QObject(sshInterface)
     , q(sshInterface)
+    , m_device(devicePrivate->q->sharedFromThis())
     , m_process(this)
     , m_devicePrivate(devicePrivate)
-    , m_device(m_devicePrivate->q->sharedFromThis())
 {
     connect(&m_process, &QtcProcess::started, this, &SshProcessInterfacePrivate::handleStarted);
     connect(&m_process, &QtcProcess::done, this, &SshProcessInterfacePrivate::handleDone);
@@ -679,22 +686,6 @@ void SshProcessInterfacePrivate::start()
     }
 }
 
-QString SshProcessInterface::pidArgumentForKill() const
-{
-    return QString::fromLatin1("-%1 %1").arg(d->m_processId);
-}
-
-void SshProcessInterfacePrivate::sendControlSignal(ControlSignal controlSignal)
-{
-    QTC_ASSERT(controlSignal != ControlSignal::KickOff, return);
-    // TODO: In case if m_processId == 0 try sending a signal based on process name.
-    const QString args = QString::fromLatin1("-%1 %2")
-            .arg(ProcessInterface::controlSignalToInt(controlSignal)).arg(q->pidArgumentForKill());
-    CommandLine command = { "kill", args, CommandLine::Raw };
-    // Note: This blocking call takes up to 2 ms for local remote.
-    m_devicePrivate->runInShell(command);
-}
-
 void SshProcessInterfacePrivate::handleConnected(const QString &socketFilePath)
 {
     m_connecting = false;
@@ -1128,7 +1119,7 @@ bool LinuxDevice::handlesFile(const FilePath &filePath) const
 
 ProcessInterface *LinuxDevice::createProcessInterface() const
 {
-    return new LinuxProcessImpl(this);
+    return new LinuxProcessInterface(this);
 }
 
 LinuxDevicePrivate::LinuxDevicePrivate(LinuxDevice *parent)

src/plugins/remotelinux/linuxprocessinterface.h

@@ -0,0 +1,55 @@
+/****************************************************************************
+**
+** Copyright (C) 2022 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+****************************************************************************/
+
+#pragma once
+
+#include "remotelinux_export.h"
+
+#include "sshprocessinterface.h"
+
+namespace RemoteLinux {
+
+class LinuxDevice;
+class SshProcessInterfacePrivate;
+
+class REMOTELINUX_EXPORT LinuxProcessInterface : public SshProcessInterface
+{
+public:
+    LinuxProcessInterface(const LinuxDevice *linuxDevice);
+    ~LinuxProcessInterface();
+
+private:
+    void sendControlSignal(Utils::ControlSignal controlSignal) override;
+
+    void handleStarted(qint64 processId) final;
+    void handleReadyReadStandardOutput(const QByteArray &outputData) final;
+
+    QString fullCommandLine(const Utils::CommandLine &commandLine) const final;
+
+    QByteArray m_output;
+    bool m_pidParsed = false;
+};
+
+} // namespace RemoteLinux

src/plugins/remotelinux/remotelinux.qbs

@@ -42,6 +42,7 @@ Project {
             "linuxdeviceprocess.h",
             "linuxdevicetester.cpp",
             "linuxdevicetester.h",
+            "linuxprocessinterface.h",
             "makeinstallstep.cpp",
             "makeinstallstep.h",
             "packageuploader.cpp",

src/plugins/remotelinux/sshprocessinterface.h

@@ -36,8 +36,6 @@ class SshProcessInterfacePrivate;
 
 class REMOTELINUX_EXPORT SshProcessInterface : public Utils::ProcessInterface
 {
-    Q_OBJECT
-
 public:
     SshProcessInterface(const LinuxDevice *linuxDevice);
     ~SshProcessInterface();
@@ -47,17 +45,18 @@ protected:
     // To be called from leaf destructor.
     // Can't call it from SshProcessInterface destructor as it calls virtual method.
     void killIfRunning();
+    qint64 processId() const;
+    bool runInShell(const Utils::CommandLine &command, const QByteArray &data = {});
 
 private:
     virtual void handleStarted(qint64 processId);
     virtual void handleReadyReadStandardOutput(const QByteArray &outputData);
 
     virtual QString fullCommandLine(const Utils::CommandLine &commandLine) const = 0;
-    virtual QString pidArgumentForKill() const;
 
     void start() final;
     qint64 write(const QByteArray &data) final;
-    void sendControlSignal(Utils::ControlSignal controlSignal) final;
+    void sendControlSignal(Utils::ControlSignal controlSignal) override = 0;
 
     bool waitForStarted(int msecs) final;
     bool waitForReadyRead(int msecs) final;