SshDeviceProcess: Fix handling finished() signal

We can't disconnect all signals inside
SshDeviceProcessPrivate::setState() since it will also
disconnect all external slots from *this signals.
The original intention was to just disconnect from
finished() signal to *this slot.

Currently there are two connections to finished() signal:
one is internal and one external. In order to guarantee
that the internal one is always invoked before the external
one, we need to do the internal connection first - so we
move the connection to the constructor. In addition we
introduce the ignoreFinished flag. The true value means
the connection is disconnected. So, instead of connecting
and disconnecting dynamically we change the flag accordingly.

In case when handleThisProcessFinished() was called we
don't emit a finised() signal from handleProcessFinished()
since this signal is going to be delivered for all other
external slots anyway after handling of
handleThisProcessFinished() finishes.

Amends 9ec997b376

Change-Id: Iae12c10251de242391c03d185a0856f421b99fe4
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/projectexplorer/devicesupport/sshdeviceprocess.cpp

@@ -51,6 +51,7 @@ public:
     SshDeviceProcessPrivate(SshDeviceProcess *q) : q(q) {}
 
     SshDeviceProcess * const q;
+    bool ignoreFinished = true;
     QSsh::SshConnection *connection = nullptr;
     QSsh::SshRemoteProcessPtr remoteProcess;
     Runnable runnable;
@@ -73,6 +74,7 @@ SshDeviceProcess::SshDeviceProcess(const IDevice::ConstPtr &device, QObject *par
     : DeviceProcess(device, QtcProcess::TerminalOn, parent),
       d(std::make_unique<SshDeviceProcessPrivate>(this))
 {
+    connect(this, &QtcProcess::finished, this, &SshDeviceProcess::handleThisProcessFinished);
     connect(&d->killTimer, &QTimer::timeout, this, &SshDeviceProcess::handleKillOperationTimeout);
 }
 
@@ -184,8 +186,7 @@ void SshDeviceProcess::handleConnected()
     if (!display.isEmpty())
         d->remoteProcess->requestX11Forwarding(display);
     if (runInTerminal()) {
-        connect(this, &QtcProcess::finished,
+        d->ignoreFinished = false;
-                this, [this] { handleProcessFinished(errorString()); });
         setAbortOnMetaChars(false);
         setCommand(d->remoteProcess->fullLocalCommandLine(true));
         QtcProcess::start();
@@ -193,7 +194,7 @@ void SshDeviceProcess::handleConnected()
         connect(d->remoteProcess.get(), &QSsh::SshRemoteProcess::started,
                 this, &SshDeviceProcess::handleProcessStarted);
         connect(d->remoteProcess.get(), &QSsh::SshRemoteProcess::done,
-                this, &SshDeviceProcess::handleProcessFinished);
+                this, &SshDeviceProcess::handleRemoteProcessFinished);
         connect(d->remoteProcess.get(), &QSsh::SshRemoteProcess::readyReadStandardOutput,
                 this, &QtcProcess::readyReadStandardOutput);
         connect(d->remoteProcess.get(), &QSsh::SshRemoteProcess::readyReadStandardError,
@@ -236,13 +237,29 @@ void SshDeviceProcess::handleProcessStarted()
     emit started();
 }
 
-void SshDeviceProcess::handleProcessFinished(const QString &error)
+void SshDeviceProcess::handleThisProcessFinished()
+{
+    if (d->ignoreFinished)
+        return;
+    // Hack: we rely on fact that this slot was called before any other external slot connected
+    // to finished() signal. That's why we don't emit finished() signal from inside
+    // handleProcessFinished() since this signal will reach all other external slots anyway.
+    handleProcessFinished(QtcProcess::errorString(), false);
+}
+
+void SshDeviceProcess::handleRemoteProcessFinished(const QString &error)
+{
+    handleProcessFinished(error, true);
+}
+
+void SshDeviceProcess::handleProcessFinished(const QString &error, bool emitFinished)
 {
     d->errorMessage = error;
     if (d->killOperation && error.isEmpty())
         d->errorMessage = tr("The process was ended forcefully.");
     d->setState(SshDeviceProcessPrivate::Inactive);
-    emit finished();
+    if (emitFinished)
+        emit finished();
 }
 
 void SshDeviceProcess::handleKillOperationFinished(const QString &errorMessage)
@@ -328,7 +345,7 @@ void SshDeviceProcess::SshDeviceProcessPrivate::setState(SshDeviceProcess::SshDe
             QMetaObject::invokeMethod(q, &QtcProcess::stopProcess, Qt::QueuedConnection);
     }
     killTimer.stop();
-    q->disconnect();
+    ignoreFinished = true;
     if (remoteProcess)
         remoteProcess->disconnect(q);
     if (connection) {

src/plugins/projectexplorer/devicesupport/sshdeviceprocess.h

@@ -60,7 +60,9 @@ private:
     void handleConnectionError();
     void handleDisconnected();
     void handleProcessStarted();
-    void handleProcessFinished(const QString &error);
+    void handleThisProcessFinished();
+    void handleRemoteProcessFinished(const QString &error);
+    void handleProcessFinished(const QString &error, bool emitFinished);
     void handleKillOperationFinished(const QString &errorMessage);
     void handleKillOperationTimeout();