docker: Change Settings to single owner

We change DockerSettings to have a single owner.
Since DockerDevices are destroyed after the plugin
is unloaded, we have to make sure to remove the
settings from devices during plugin teardown.

For this we store a list of created devices in
the factory, and call their shutdown function when
the plugin unloads.

Change-Id: Ic9c7d8ad9437c48d68f20c9a8f8ad7449b3cb972
Reviewed-by: hjk <[email protected]>

7 files changed, 68 insertions, 33 deletions

diff --git a/src/plugins/docker/dockerapi.cpp b/src/plugins/docker/dockerapi.cpp
index 0acf60f3f97..af9dc79661f 100644
--- a/src/plugins/docker/dockerapi.cpp
+++ b/src/plugins/docker/dockerapi.cpp
@@ -43,7 +43,7 @@ using namespace Utils;
 
 DockerApi *s_instance{nullptr};
 
-DockerApi::DockerApi(QSharedPointer<DockerSettings> settings)
+DockerApi::DockerApi(DockerSettings *settings)
     : m_settings(settings)
 {
     s_instance = this;
diff --git a/src/plugins/docker/dockerapi.h b/src/plugins/docker/dockerapi.h
index 96f5576871c..815caff2ecb 100644
--- a/src/plugins/docker/dockerapi.h
+++ b/src/plugins/docker/dockerapi.h
@@ -42,7 +42,7 @@ class DockerApi : public QObject
     Q_OBJECT
 
 public:
-    DockerApi(QSharedPointer<DockerSettings> settings);
+    DockerApi(DockerSettings *settings);
 
     static DockerApi *instance();
 
@@ -63,7 +63,7 @@ private:
 private:
     Utils::optional<bool> m_dockerDaemonAvailable;
     QMutex m_daemonCheckGuard;
-    QSharedPointer<DockerSettings> m_settings;
+    DockerSettings *m_settings;
 };
 
 } // namespace Internal
diff --git a/src/plugins/docker/dockerdevice.cpp b/src/plugins/docker/dockerdevice.cpp
index 918a5054969..ee47708e392 100644
--- a/src/plugins/docker/dockerdevice.cpp
+++ b/src/plugins/docker/dockerdevice.cpp
@@ -106,7 +106,7 @@ static Q_LOGGING_CATEGORY(dockerDeviceLog, "qtc.docker.device", QtWarningMsg);
 class ContainerShell : public Utils::DeviceShell
 {
 public:
-    ContainerShell(QSharedPointer<DockerSettings> settings, const QString &containerId)
+    ContainerShell(DockerSettings *settings, const QString &containerId)
         : m_settings(settings)
         , m_containerId(containerId)
     {
@@ -119,7 +119,7 @@ private:
     }
 
 private:
-    QSharedPointer<DockerSettings> m_settings;
+    DockerSettings *m_settings;
     QString m_containerId;
 };
 
@@ -128,7 +128,7 @@ class DockerDevicePrivate : public QObject
     Q_DECLARE_TR_FUNCTIONS(Docker::Internal::DockerDevice)
 
 public:
-    DockerDevicePrivate(DockerDevice *parent, QSharedPointer<DockerSettings> settings)
+    DockerDevicePrivate(DockerDevice *parent, DockerSettings *settings)
         : q(parent)
         , m_settings(settings)
     {}
@@ -147,7 +147,7 @@ public:
 
     DockerDevice *q;
     DockerDeviceData m_data;
-    QSharedPointer<DockerSettings> m_settings;
+    DockerSettings *m_settings;
 
     // For local file access
 
@@ -320,7 +320,7 @@ QString DockerDeviceData::repoAndTag() const
 
 // DockerDevice
 
-DockerDevice::DockerDevice(QSharedPointer<DockerSettings> settings, const DockerDeviceData &data)
+DockerDevice::DockerDevice(DockerSettings *settings, const DockerDeviceData &data)
     : d(new DockerDevicePrivate(this, settings))
 {
     d->m_data = data;
@@ -364,6 +364,12 @@ DockerDevice::~DockerDevice()
     delete d;
 }
 
+void DockerDevice::shutdown()
+{
+    d->stopCurrentContainer();
+    d->m_settings = nullptr;
+}
+
 const DockerDeviceData &DockerDevice::data() const
 {
     return d->m_data;
@@ -374,8 +380,6 @@ DockerDeviceData &DockerDevice::data()
     return d->m_data;
 }
 
-
-
 void DockerDevice::updateContainerAccess() const
 {
     d->updateContainerAccess();
@@ -383,7 +387,7 @@ void DockerDevice::updateContainerAccess() const
 
 void DockerDevicePrivate::stopCurrentContainer()
 {
-    if (m_container.isEmpty() || !DockerApi::isDockerDaemonAvailable(false).value_or(false))
+    if (!m_settings || m_container.isEmpty() || !DockerApi::isDockerDaemonAvailable(false).value_or(false))
         return;
 
     m_shell.reset();
@@ -412,6 +416,9 @@ static QString getLocalIPv4Address()
 
 void DockerDevicePrivate::startContainer()
 {
+    if (!m_settings)
+        return;
+
     const QString display = HostOsInfo::isLinuxHost() ? QString(":0")
                                                       : QString(getLocalIPv4Address() + ":0.0");
     CommandLine dockerCreate{m_settings->dockerBinaryPath.filePath(), {"create",
@@ -494,6 +501,9 @@ void DockerDevice::setMounts(const QStringList &mounts) const
 
 CommandLine DockerDevice::withDockerExecCmd(const Utils::CommandLine &cmd, bool interactive) const
 {
+    if (!d->m_settings)
+        return {};
+
     QStringList args;
 
     args << "exec";
@@ -1003,8 +1013,9 @@ void DockerDevicePrivate::fetchSystemEnviroment()
 
 bool DockerDevicePrivate::runInContainer(const CommandLine &cmd) const
 {
-    if (!DockerApi::isDockerDaemonAvailable(false).value_or(false))
+    if (!m_settings || !DockerApi::isDockerDaemonAvailable(false).value_or(false))
         return false;
+
     CommandLine dcmd{m_settings->dockerBinaryPath.filePath(), {"exec", m_container}};
     dcmd.addCommandLineAsArgs(cmd);
 
@@ -1066,7 +1077,7 @@ public:
 class DockerDeviceSetupWizard final : public QDialog
 {
 public:
-    DockerDeviceSetupWizard(QSharedPointer<DockerSettings> settings)
+    DockerDeviceSetupWizard(DockerSettings *settings)
         : QDialog(ICore::dialogParent())
         , m_settings(settings)
     {
@@ -1168,7 +1179,7 @@ public:
     TreeView *m_view = nullptr;
     QTextBrowser *m_log = nullptr;
     QDialogButtonBox *m_buttons;
-    QSharedPointer<DockerSettings> m_settings;
+    DockerSettings *m_settings;
 
     QtcProcess *m_process = nullptr;
     QString m_selectedId;
@@ -1176,7 +1187,7 @@ public:
 
 // Factory
 
-DockerDeviceFactory::DockerDeviceFactory(QSharedPointer<DockerSettings> settings)
+DockerDeviceFactory::DockerDeviceFactory(DockerSettings *settings)
     : IDeviceFactory(Constants::DOCKER_DEVICE_TYPE)
 {
     setDisplayName(DockerDevice::tr("Docker Device"));
@@ -1187,7 +1198,21 @@ DockerDeviceFactory::DockerDeviceFactory(QSharedPointer<DockerSettings> settings
             return IDevice::Ptr();
         return wizard.device();
     });
-    setConstructionFunction([settings] { return DockerDevice::create(settings, {}); });
+    setConstructionFunction([settings, this] {
+        auto device = DockerDevice::create(settings, {});
+        QMutexLocker lk(&m_deviceListMutex);
+        m_existingDevices.push_back(device);
+        return device;
+    });
+}
+
+void DockerDeviceFactory::shutdownExistingDevices()
+{
+    QMutexLocker lk(&m_deviceListMutex);
+    for (const auto &weakDevice : m_existingDevices) {
+        if (QSharedPointer<DockerDevice> device = weakDevice.lock())
+            device->shutdown();
+    }
 }
 
 } // Internal
diff --git a/src/plugins/docker/dockerdevice.h b/src/plugins/docker/dockerdevice.h
index 9ddef267e51..16fe4ef9dc7 100644
--- a/src/plugins/docker/dockerdevice.h
+++ b/src/plugins/docker/dockerdevice.h
@@ -33,6 +33,8 @@
 
 #include <utils/aspects.h>
 
+#include <QMutex>
+
 namespace Docker {
 namespace Internal {
 
@@ -58,10 +60,12 @@ public:
     using Ptr = QSharedPointer<DockerDevice>;
     using ConstPtr = QSharedPointer<const DockerDevice>;
 
-    explicit DockerDevice(QSharedPointer<DockerSettings> settings, const DockerDeviceData &data);
+    explicit DockerDevice(DockerSettings *settings, const DockerDeviceData &data);
     ~DockerDevice();
 
-    static Ptr create(QSharedPointer<DockerSettings> settings, const DockerDeviceData &data) { return Ptr(new DockerDevice(settings, data)); }
+    void shutdown();
+
+    static Ptr create(DockerSettings *settings, const DockerDeviceData &data) { return Ptr(new DockerDevice(settings, data)); }
 
     ProjectExplorer::IDeviceWidget *createWidget() override;
     QList<ProjectExplorer::Task> validate() const override;
@@ -135,7 +139,13 @@ private:
 class DockerDeviceFactory final : public ProjectExplorer::IDeviceFactory
 {
 public:
-    DockerDeviceFactory(QSharedPointer<DockerSettings> settings);
+    DockerDeviceFactory(DockerSettings *settings);
+
+    void shutdownExistingDevices();
+
+private:
+    QMutex m_deviceListMutex;
+    std::vector<QWeakPointer<DockerDevice> > m_existingDevices;
 };
 
 } // Internal
diff --git a/src/plugins/docker/dockerplugin.cpp b/src/plugins/docker/dockerplugin.cpp
index fb3720358f2..71e9837a4fb 100644
--- a/src/plugins/docker/dockerplugin.cpp
+++ b/src/plugins/docker/dockerplugin.cpp
@@ -43,14 +43,14 @@ namespace Internal {
 class DockerPluginPrivate
 {
 public:
-    QSharedPointer<DockerSettings> m_settings{new DockerSettings};
-    DockerDeviceFactory deviceFactory{m_settings};
-    DockerSettingsPage m_settingPage{m_settings};
-
-    // DockerBuildStepFactory buildStepFactory;
-    Utils::optional<bool> daemonRunning;
-
-    DockerApi dockerApi{m_settings};
+    ~DockerPluginPrivate() {
+        m_deviceFactory.shutdownExistingDevices();
+    }
+
+    DockerSettings m_settings;
+    DockerDeviceFactory m_deviceFactory{&m_settings};
+    DockerSettingsPage m_settingPage{&m_settings};
+    DockerApi m_dockerApi{&m_settings};
 };
 
 static DockerPlugin *s_instance = nullptr;
@@ -63,7 +63,7 @@ DockerPlugin::DockerPlugin()
 DockerApi *DockerPlugin::dockerApi()
 {
     QTC_ASSERT(s_instance, return nullptr);
-    return &s_instance->d->dockerApi;
+    return &s_instance->d->m_dockerApi;
 }
 
 DockerPlugin::~DockerPlugin()
diff --git a/src/plugins/docker/dockersettings.cpp b/src/plugins/docker/dockersettings.cpp
index a752bc0aea2..3cab53e3e76 100644
--- a/src/plugins/docker/dockersettings.cpp
+++ b/src/plugins/docker/dockersettings.cpp
@@ -47,7 +47,7 @@ DockerSettings::DockerSettings()
     registerAspect(&dockerBinaryPath);
     dockerBinaryPath.setDisplayStyle(StringAspect::PathChooserDisplay);
     dockerBinaryPath.setExpectedKind(PathChooser::ExistingCommand);
-    dockerBinaryPath.setDefaultValue(FilePath::fromString("docker").searchInPath().toString());
+    dockerBinaryPath.setDefaultValue(FilePath::fromString("docker").searchInPath({"/usr/local/bin"}).toString());
     dockerBinaryPath.setDisplayName(tr("Docker CLI"));
     dockerBinaryPath.setHistoryCompleter("Docker.Command.History");
     dockerBinaryPath.setLabelText(tr("Command:"));
@@ -58,14 +58,14 @@ DockerSettings::DockerSettings()
 
 // DockerSettingsPage
 
-DockerSettingsPage::DockerSettingsPage(QSharedPointer<DockerSettings> settings)
+DockerSettingsPage::DockerSettingsPage(DockerSettings *settings)
 {
     setId(Docker::Constants::DOCKER_SETTINGS_ID);
     setDisplayName(DockerSettings::tr("Docker"));
     setCategory(ProjectExplorer::Constants::DEVICE_SETTINGS_CATEGORY);
-    setSettings(settings.get());
+    setSettings(settings);
 
-    setLayouter([settings = settings.get()](QWidget *widget) {
+    setLayouter([settings](QWidget *widget) {
         DockerSettings &s = *settings;
         using namespace Layouting;
 
diff --git a/src/plugins/docker/dockersettings.h b/src/plugins/docker/dockersettings.h
index 88d590d00ce..f5c535c5678 100644
--- a/src/plugins/docker/dockersettings.h
+++ b/src/plugins/docker/dockersettings.h
@@ -44,7 +44,7 @@ public:
 class DockerSettingsPage final : public Core::IOptionsPage
 {
 public:
-    explicit DockerSettingsPage(QSharedPointer<DockerSettings> settings);
+    explicit DockerSettingsPage(DockerSettings *settings);
 };
 
 } // namespace Internal