LanguageClient: Fix crash after disabling client setting

Change-Id: I0f9ec02edc645726764fc9ea9ea1832e40bbef80
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/languageclient/languageclientmanager.cpp

@@ -126,7 +126,7 @@ void LanguageClientManager::startClient(BaseSettings *setting, ProjectExplorer::
     QTC_ASSERT(client, return);
     client->setCurrentProject(project);
     startClient(client);
-    managerInstance->m_clientsForSetting[setting->m_id].append(QPointer<Client>(client));
+    managerInstance->m_clientsForSetting[setting->m_id].append(client);
 }
 
 QVector<Client *> LanguageClientManager::clients()
@@ -151,12 +151,24 @@ void LanguageClientManager::reportFinished(const MessageId &id, Client *byClient
     managerInstance->m_exclusiveRequests.remove(id);
 }
 
+void LanguageClientManager::shutdownClient(Client *client)
+{
+    if (!client)
+        return;
+    if (client->reachable())
+        client->shutdown();
+    else if (client->state() != Client::Shutdown && client->state() != Client::ShutdownRequested)
+        deleteClient(client);
+}
+
 void LanguageClientManager::deleteClient(Client *client)
 {
     QTC_ASSERT(managerInstance, return);
     QTC_ASSERT(client, return);
     client->disconnect();
     managerInstance->m_clients.removeAll(client);
+    for (QVector<Client *> &clients : managerInstance->m_clientsForSetting)
+        clients.removeAll(client);
     if (managerInstance->m_shuttingDown)
         delete client;
     else
@@ -169,12 +181,8 @@ void LanguageClientManager::shutdown()
     if (managerInstance->m_shuttingDown)
         return;
     managerInstance->m_shuttingDown = true;
-    for (auto interface : managerInstance->m_clients) {
+    for (Client *client : managerInstance->m_clients)
-        if (interface->reachable())
+        shutdownClient(client);
-            interface->shutdown();
-        else
-            deleteClient(interface);
-    }
     QTimer::singleShot(3000, managerInstance, [](){
         for (auto interface : managerInstance->m_clients)
             deleteClient(interface);
@@ -213,12 +221,8 @@ void LanguageClientManager::applySettings()
     });
 
     for (BaseSettings *setting : restarts) {
-        for (const QPointer<Client> &client : clientForSetting(setting)) {
+        for (Client *client : clientForSetting(setting))
-            if (client->reachable())
+            shutdownClient(client);
-                client->shutdown();
-            else
-                deleteClient(client);
-        }
         if (!setting->isValid() || !setting->m_enabled)
             continue;
         switch (setting->m_startBehavior) {
@@ -258,17 +262,18 @@ QList<BaseSettings *> LanguageClientManager::currentSettings()
     return managerInstance->m_currentSettings;
 }
 
-QVector<QPointer<Client>> LanguageClientManager::clientForSetting(const BaseSettings *setting)
+QVector<Client *> LanguageClientManager::clientForSetting(const BaseSettings *setting)
 {
     QTC_ASSERT(managerInstance, return {});
-    return managerInstance->m_clientsForSetting.value(setting->m_id);
+    auto instance = managerInstance;
+    return instance->m_clientsForSetting.value(setting->m_id);
 }
 
 const BaseSettings *LanguageClientManager::settingForClient(Client *client)
 {
     QTC_ASSERT(managerInstance, return nullptr);
     for (const QString &id : managerInstance->m_clientsForSetting.keys()) {
-        for (const QPointer<Client> &settingClient : managerInstance->m_clientsForSetting[id]) {
+        for (const Client *settingClient : managerInstance->m_clientsForSetting[id]) {
             if (settingClient == client) {
                 return Utils::findOrDefault(managerInstance->m_currentSettings,
                                             [id](BaseSettings *setting) {
@@ -365,7 +370,7 @@ void LanguageClientManager::documentOpened(Core::IDocument *document)
 {
     // check whether we have to start servers for this document
     for (BaseSettings *setting : LanguageClientSettings::currentPageSettings()) {
-        const QVector<QPointer<Client>> clients = clientForSetting(setting);
+        const QVector<Client *> clients = clientForSetting(setting);
         if (setting->isValid() && setting->m_enabled
             && setting->m_languageFilter.isSupported(document)) {
             if (setting->m_startBehavior == BaseSettings::RequiresProject) {
@@ -522,7 +527,7 @@ void LanguageClientManager::projectAdded(ProjectExplorer::Project *project)
                                      [project](QPointer<Client> client) {
                                          return client->project() == project;
                                      })
-                    .isNull()) {
+                    == nullptr) {
                 for (Core::IDocument *doc : Core::DocumentModel::openedDocuments()) {
                     if (setting->m_languageFilter.isSupported(doc)) {
                         if (project->isKnownFile(doc->filePath()))

src/plugins/languageclient/languageclientmanager.h

@@ -63,6 +63,7 @@ public:
     static void addExclusiveRequest(const LanguageServerProtocol::MessageId &id, Client *client);
     static void reportFinished(const LanguageServerProtocol::MessageId &id, Client *byClient);
 
+    static void shutdownClient(Client *client);
     static void deleteClient(Client *client);
 
     static void shutdown();
@@ -73,7 +74,7 @@ public:
 
     static void applySettings();
     static QList<BaseSettings *> currentSettings();
-    static QVector<QPointer<Client> > clientForSetting(const BaseSettings *setting);
+    static QVector<Client *> clientForSetting(const BaseSettings *setting);
     static const BaseSettings *settingForClient(Client *setting);
     static Client *clientForEditor(Core::IEditor *editor);
 
@@ -103,7 +104,7 @@ private:
     bool m_shuttingDown = false;
     QVector<Client *> m_clients;
     QList<BaseSettings *>  m_currentSettings; // owned
-    QMap<QString, QVector<QPointer<Client>>> m_clientsForSetting;
+    QMap<QString, QVector<Client *>> m_clientsForSetting;
     QHash<LanguageServerProtocol::MessageId, QList<Client *>> m_exclusiveRequests;
     DocumentLocatorFilter m_currentDocumentLocatorFilter;
 };

src/plugins/languageclient/languageclientsettings.cpp

@@ -262,12 +262,8 @@ void LanguageClientSettingsPage::apply()
     LanguageClientManager::applySettings();
 
     for (BaseSettings *setting : m_model.removed()) {
-        for (Client *client : LanguageClientManager::clientForSetting(setting)) {
+        for (Client *client : LanguageClientManager::clientForSetting(setting))
-            if (client->reachable())
+            LanguageClientManager::shutdownClient(client);
-                client->shutdown();
-            else
-                LanguageClientManager::deleteClient(client);
-        }
     }
 
     if (m_widget) {
@@ -386,12 +382,12 @@ QWidget *BaseSettings::createSettingsWidget(QWidget *parent) const
 
 bool BaseSettings::needsRestart() const
 {
-    const QVector<QPointer<Client>> clients = LanguageClientManager::clientForSetting(this);
+    const QVector<Client *> clients = LanguageClientManager::clientForSetting(this);
     if (clients.isEmpty())
         return m_enabled;
     if (!m_enabled)
         return true;
-    return Utils::anyOf(clients, [this](const QPointer<Client> &client) {
+    return Utils::anyOf(clients, [this](const Client *client) {
         return client->needsRestart(this);
     });
 }
@@ -603,7 +599,7 @@ BaseSettingsWidget::BaseSettingsWidget(const BaseSettings *settings, QWidget *pa
     };
 
     mainLayout->addWidget(new QLabel(tr("Capabilities:")), ++row, 0, Qt::AlignTop);
-    QVector<QPointer<Client> > clients = LanguageClientManager::clientForSetting(settings);
+    QVector<Client *> clients = LanguageClientManager::clientForSetting(settings);
     if (clients.isEmpty()) {
         mainLayout->addWidget(createInfoLabel());
     } else { // TODO move the capabilities view into a new widget outside of the settings