Control the lifetime of SshProcessManager

Make it possible to control when the SshProcessManager is
being constructed / destructed. Expose public constructor.

Since different singletons depend on each other, we need
to control the order of creation and destruction of them.

The order of creation is like that:

1. QCoreApplication
2. ProcessReaper
3. ProcessLauncher
4. SshConnectionManager

The order of destruction must be opposite to the above.

Change-Id: Ice07eb751cd61c03cb461816fa1b74ab040a53de
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/app/CMakeLists.txt

@@ -8,7 +8,7 @@ install(
 
 add_qtc_executable(qtcreator
   DEFINES IDE_LIBRARY_BASENAME=\"${IDE_LIBRARY_BASE_PATH}\"
-  DEPENDS Aggregation ExtensionSystem Qt5::Core Qt5::Widgets Utils shared_qtsingleapplication app_version
+  DEPENDS Aggregation ExtensionSystem Qt5::Core Qt5::Widgets Utils QtcSsh shared_qtsingleapplication app_version
   SOURCES
     main.cpp
     ../tools/qtcreatorcrashhandler/crashhandlersetup.cpp ../tools/qtcreatorcrashhandler/crashhandlersetup.h

src/app/app.pro

@@ -17,7 +17,7 @@ include(../rpath.pri)
 
 include(../libs/qt-breakpad/qtbreakpad.pri)
 
-LIBS *= -l$$qtLibraryName(ExtensionSystem) -l$$qtLibraryName(Aggregation) -l$$qtLibraryName(Utils)
+LIBS *= -l$$qtLibraryName(ExtensionSystem) -l$$qtLibraryName(Aggregation) -l$$qtLibraryName(Utils) -l$$qtLibraryName(QtcSsh)
 
 win32 {
     # We need the version in two separate formats for the .rc file

src/app/app.qbs

@@ -46,6 +46,7 @@ QtcProduct {
     Depends { name: "app_version_header" }
     Depends { name: "Qt"; submodules: ["widgets", "network"] }
     Depends { name: "Utils" }
+    Depends { name: "QtcSsh" }
     Depends { name: "ExtensionSystem" }
 
     files: [

src/app/main.cpp

@@ -32,6 +32,8 @@
 #include <extensionsystem/pluginspec.h>
 #include <qtsingleapplication.h>
 
+#include <ssh/sshconnectionmanager.h>
+
 #include <utils/algorithm.h>
 #include <utils/environment.h>
 #include <utils/fileutils.h>
@@ -538,6 +540,7 @@ int main(int argc, char **argv)
     Utils::ProcessReaper processReaper;
     Utils::LauncherInterface::startLauncher();
     auto cleanup = qScopeGuard([] { Utils::LauncherInterface::stopLauncher(); });
+    QSsh::SshConnectionManager sshConnectionManager;
 
     const QStringList pluginArguments = app.arguments();
 

src/libs/ssh/sftpfilesystemmodel.cpp

@@ -113,7 +113,7 @@ SftpFileSystemModel::~SftpFileSystemModel()
 void SftpFileSystemModel::setSshConnection(const SshConnectionParameters &sshParams)
 {
     QTC_ASSERT(!d->sshConnection, return);
-    d->sshConnection = QSsh::acquireConnection(sshParams);
+    d->sshConnection = SshConnectionManager::acquireConnection(sshParams);
     connect(d->sshConnection, &SshConnection::errorOccurred,
             this, &SftpFileSystemModel::handleSshConnectionFailure);
     if (d->sshConnection->state() == SshConnection::Connected) {
@@ -268,7 +268,7 @@ void SftpFileSystemModel::shutDown()
     }
     if (d->sshConnection) {
         disconnect(d->sshConnection, nullptr, this, nullptr);
-        QSsh::releaseConnection(d->sshConnection);
+        SshConnectionManager::releaseConnection(d->sshConnection);
         d->sshConnection = nullptr;
     }
     delete d->rootNode;

src/libs/ssh/sshconnectionmanager.cpp

@@ -38,6 +38,7 @@
 
 namespace QSsh {
 namespace Internal {
+
 class UnacquiredConnection {
 public:
     UnacquiredConnection(SshConnection *conn) : connection(conn), scheduledForRemoval(false) {}
@@ -52,21 +53,25 @@ bool operator!=(const UnacquiredConnection &c1, const UnacquiredConnection &c2)
     return !(c1 == c2);
 }
 
-static class SshConnectionManager *s_instance = nullptr;
+static class SshConnectionManagerPrivate *s_instance = nullptr;
 
-class SshConnectionManager : public QObject
+class SshConnectionManagerPrivate : public QObject
 {
 public:
-    SshConnectionManager(QObject *parent)
+    SshConnectionManagerPrivate()
-        : QObject(parent)
     {
+        QTC_ASSERT(s_instance == nullptr, return);
+        s_instance = this;
+
         connect(&m_removalTimer, &QTimer::timeout,
-                this, &SshConnectionManager::removeInactiveConnections);
+                this, &SshConnectionManagerPrivate::removeInactiveConnections);
         m_removalTimer.start(SshSettings::connectionSharingTimeout() * 1000 * 60 / 2);
     }
 
-    ~SshConnectionManager()
+    ~SshConnectionManagerPrivate() override
     {
+        QTC_ASSERT(s_instance == this, return);
+
         for (const UnacquiredConnection &connection : qAsConst(m_unacquiredConnections)) {
             disconnect(connection.connection, nullptr, this, nullptr);
             delete connection.connection;
@@ -110,7 +115,7 @@ public:
         // create a new connection:
         SshConnection * const connection = new SshConnection(sshParams);
         connect(connection, &SshConnection::disconnected,
-                this, &SshConnectionManager::cleanup);
+                this, &SshConnectionManagerPrivate::cleanup);
         if (SshSettings::connectionSharingEnabled())
             m_acquiredConnections.append(connection);
 
@@ -203,29 +208,34 @@ private:
     QTimer m_removalTimer;
 };
 
-static SshConnectionManager *instance()
-{
-    QTC_CHECK(QThread::currentThread() == qApp->thread());
-    if (s_instance == nullptr)
-        s_instance = new SshConnectionManager(qApp); // Deleted together with application
-    return s_instance;
-}
-
 } // namespace Internal
 
-SshConnection *acquireConnection(const SshConnectionParameters &sshParams)
+SshConnectionManager::SshConnectionManager()
+    : d(new Internal::SshConnectionManagerPrivate())
 {
-    return Internal::instance()->acquireConnection(sshParams);
 }
 
-void releaseConnection(SshConnection *connection)
+SshConnectionManager::~SshConnectionManager()
 {
-    Internal::instance()->releaseConnection(connection);
+    delete d;
 }
 
-void forceNewConnection(const SshConnectionParameters &sshParams)
+SshConnection *SshConnectionManager::acquireConnection(const SshConnectionParameters &sshParams)
 {
-    Internal::instance()->forceNewConnection(sshParams);
+    QTC_ASSERT(Internal::s_instance, return nullptr);
+    return Internal::s_instance->acquireConnection(sshParams);
+}
+
+void SshConnectionManager::releaseConnection(SshConnection *connection)
+{
+    QTC_ASSERT(Internal::s_instance, return);
+    Internal::s_instance->releaseConnection(connection);
+}
+
+void SshConnectionManager::forceNewConnection(const SshConnectionParameters &sshParams)
+{
+    QTC_ASSERT(Internal::s_instance, return);
+    Internal::s_instance->forceNewConnection(sshParams);
 }
 
 } // namespace QSsh

src/libs/ssh/sshconnectionmanager.h

@@ -32,10 +32,21 @@ namespace QSsh {
 class SshConnection;
 class SshConnectionParameters;
 
-QSSH_EXPORT SshConnection *acquireConnection(const SshConnectionParameters &sshParams);
+namespace Internal { class SshConnectionManagerPrivate; }
-QSSH_EXPORT void releaseConnection(SshConnection *connection);
 
+class QSSH_EXPORT SshConnectionManager final
+{
+public:
+    SshConnectionManager();
+    ~SshConnectionManager();
+
+    static SshConnection *acquireConnection(const SshConnectionParameters &sshParams);
+    static void releaseConnection(SshConnection *connection);
     // Make sure the next acquireConnection with the given parameters will return a new connection.
-QSSH_EXPORT void forceNewConnection(const SshConnectionParameters &sshParams);
+    static void forceNewConnection(const SshConnectionParameters &sshParams);
+
+private:
+    Internal::SshConnectionManagerPrivate *d;
+};
 
 } // namespace QSsh

src/libs/ssh/sshremoteprocessrunner.cpp

@@ -100,7 +100,7 @@ void SshRemoteProcessRunner::runInternal(const QString &command,
     d->m_processErrorString.clear();
     d->m_exitCode = -1;
     d->m_command = command;
-    d->m_connection = QSsh::acquireConnection(sshParams);
+    d->m_connection = SshConnectionManager::acquireConnection(sshParams);
     connect(d->m_connection, &SshConnection::errorOccurred,
             this, &SshRemoteProcessRunner::handleConnectionError);
     connect(d->m_connection, &SshConnection::disconnected,
@@ -190,7 +190,7 @@ void SshRemoteProcessRunner::setState(int newState)
         }
         if (d->m_connection) {
             disconnect(d->m_connection, nullptr, this, nullptr);
-            QSsh::releaseConnection(d->m_connection);
+            SshConnectionManager::releaseConnection(d->m_connection);
             d->m_connection = nullptr;
         }
     }

src/plugins/projectexplorer/devicesupport/sshdeviceprocess.cpp

@@ -96,7 +96,7 @@ void SshDeviceProcess::start(const Runnable &runnable)
     d->runnable = runnable;
     QSsh::SshConnectionParameters params = device()->sshParameters();
     params.x11DisplayName = d->displayName();
-    d->connection = QSsh::acquireConnection(params);
+    d->connection = QSsh::SshConnectionManager::acquireConnection(params);
     connect(d->connection, &QSsh::SshConnection::errorOccurred,
             this, &SshDeviceProcess::handleConnectionError);
     connect(d->connection, &QSsh::SshConnection::disconnected,
@@ -364,7 +364,7 @@ void SshDeviceProcess::SshDeviceProcessPrivate::setState(SshDeviceProcess::SshDe
         process->disconnect(q);
     if (connection) {
         connection->disconnect(q);
-        QSsh::releaseConnection(connection);
+        QSsh::SshConnectionManager::releaseConnection(connection);
         connection = nullptr;
     }
 }

src/plugins/remotelinux/abstractremotelinuxdeployservice.cpp

@@ -194,7 +194,7 @@ void AbstractRemoteLinuxDeployService::handleDeviceSetupDone(bool success)
     }
 
     d->state = Connecting;
-    d->connection = QSsh::acquireConnection(deviceConfiguration()->sshParameters());
+    d->connection = SshConnectionManager::acquireConnection(deviceConfiguration()->sshParameters());
     connect(d->connection, &SshConnection::errorOccurred,
             this, &AbstractRemoteLinuxDeployService::handleConnectionFailure);
     if (d->connection->state() == SshConnection::Connected) {
@@ -259,7 +259,7 @@ void AbstractRemoteLinuxDeployService::setFinished()
     d->state = Inactive;
     if (d->connection) {
         disconnect(d->connection, nullptr, this, nullptr);
-        QSsh::releaseConnection(d->connection);
+        SshConnectionManager::releaseConnection(d->connection);
         d->connection = nullptr;
     }
     d->stopRequested = false;

src/plugins/remotelinux/linuxdevicetester.cpp

@@ -72,7 +72,7 @@ GenericLinuxDeviceTester::GenericLinuxDeviceTester(QObject *parent)
 GenericLinuxDeviceTester::~GenericLinuxDeviceTester()
 {
     if (d->connection)
-        releaseConnection(d->connection);
+        SshConnectionManager::releaseConnection(d->connection);
     delete d;
 }
 
@@ -81,8 +81,8 @@ void GenericLinuxDeviceTester::testDevice(const IDevice::Ptr &deviceConfiguratio
     QTC_ASSERT(d->state == Inactive, return);
 
     d->deviceConfiguration = deviceConfiguration;
-    forceNewConnection(deviceConfiguration->sshParameters());
+    SshConnectionManager::forceNewConnection(deviceConfiguration->sshParameters());
-    d->connection = acquireConnection(deviceConfiguration->sshParameters());
+    d->connection = SshConnectionManager::acquireConnection(deviceConfiguration->sshParameters());
     connect(d->connection, &SshConnection::connected,
             this, &GenericLinuxDeviceTester::handleConnected);
     connect(d->connection, &SshConnection::errorOccurred,
@@ -275,7 +275,7 @@ void GenericLinuxDeviceTester::setFinished(TestResult result)
     }
     if (d->connection) {
         disconnect(d->connection, nullptr, this, nullptr);
-        releaseConnection(d->connection);
+        SshConnectionManager::releaseConnection(d->connection);
         d->connection = nullptr;
     }
     emit finished(result);

tests/auto/ssh/tst_ssh.cpp

@@ -26,6 +26,7 @@
 #include <ssh/sftpsession.h>
 #include <ssh/sftptransfer.h>
 #include <ssh/sshconnection.h>
+#include <ssh/sshconnectionmanager.h>
 #include <ssh/sshremoteprocessrunner.h>
 #include <ssh/sshsettings.h>
 #include <utils/algorithm.h>
@@ -115,6 +116,8 @@ private slots:
 
 private:
     bool waitForConnection(SshConnection &connection);
+
+    SshConnectionManager sshConnectionManager;
 };
 
 void tst_Ssh::initTestCase()