Fix shared pointer deletion before the slot was invoked

Fixes: #9367
owncloud · Mar 1, 2022 · 83d238f · 83d238f
1 parent 14146c8
commit 83d238f
Show file tree

Hide file tree

Showing 14 changed files with 30 additions and 26 deletions.
diff --git a/changelog/unreleased/9367 b/changelog/unreleased/9367
@@ -0,0 +1,5 @@
+Bugfix: Fix crash on remove account
+
+We fixed a potential reference to a deleted item, when an account was removed.
+
+https://github.com/owncloud/client/issues/9367
diff --git a/src/gui/accountmanager.cpp b/src/gui/accountmanager.cpp
@@ -391,7 +391,7 @@ void AccountManager::deleteAccount(AccountState *account)
     auto settings = ConfigFile::settingsWithGroup(QLatin1String(accountsC));
     settings->remove(account->account()->id());
 
-    emit accountRemoved(account);
+    emit accountRemoved(copy);
 }
 
 AccountPtr AccountManager::createAccount()
@@ -407,7 +407,7 @@ void AccountManager::shutdown()
 {
     const auto accounts = std::move(_accounts);
     for (const auto &acc : accounts) {
-        emit accountRemoved(acc.data());
+        emit accountRemoved(acc);
     }
 }
 

diff --git a/src/gui/accountmanager.h b/src/gui/accountmanager.h
@@ -116,7 +116,7 @@ public slots:
 
 Q_SIGNALS:
     void accountAdded(AccountState *account);
-    void accountRemoved(AccountState *account);
+    void accountRemoved(const AccountStatePtr &s);
 
 private:
     AccountManager() {}

diff --git a/src/gui/activitywidget.cpp b/src/gui/activitywidget.cpp
@@ -90,7 +90,7 @@ ActivityWidget::ActivityWidget(QWidget *parent)
     connect(_model, &ActivityListModel::activityJobStatusCode,
         this, &ActivityWidget::slotAccountActivityStatus);
 
-    connect(AccountManager::instance(), &AccountManager::accountRemoved, this, [this](AccountState *ast) {
+    connect(AccountManager::instance(), &AccountManager::accountRemoved, this, [this](const AccountStatePtr &ast) {
         if (_accountsWithoutActivities.remove(ast->account()->displayName())) {
             showLabels();
         }
@@ -139,7 +139,7 @@ void ActivityWidget::slotRefreshNotifications(AccountState *ptr)
     }
 }
 
-void ActivityWidget::slotRemoveAccount(AccountState *ptr)
+void ActivityWidget::slotRemoveAccount(const AccountStatePtr &ptr)
 {
     _model->slotRemoveAccount(ptr);
 }
@@ -556,7 +556,7 @@ void ActivitySettings::slotShowIssuesTab()
     _tab->setCurrentIndex(_syncIssueTabId);
 }
 
-void ActivitySettings::slotRemoveAccount(AccountState *ptr)
+void ActivitySettings::slotRemoveAccount(const AccountStatePtr &ptr)
 {
     _activityWidget->slotRemoveAccount(ptr);
 }

diff --git a/src/gui/activitywidget.h b/src/gui/activitywidget.h
@@ -73,7 +73,7 @@ class ActivityWidget : public QWidget
 public slots:
     void slotRefreshActivities(AccountState *ptr);
     void slotRefreshNotifications(AccountState *ptr);
-    void slotRemoveAccount(AccountState *ptr);
+    void slotRemoveAccount(const AccountStatePtr &ptr);
     void slotAccountActivityStatus(AccountState *ast, int statusCode);
     void slotRequestCleanupAndBlacklist(const Activity &blacklistActivity);
 
@@ -134,7 +134,7 @@ class ActivitySettings : public QWidget
 
 public slots:
     void slotRefresh(AccountState *ptr);
-    void slotRemoveAccount(AccountState *ptr);
+    void slotRemoveAccount(const AccountStatePtr &ptr);
 
     void setNotificationRefreshInterval(std::chrono::milliseconds interval);
 

diff --git a/src/gui/application.cpp b/src/gui/application.cpp
@@ -386,16 +386,16 @@ Application::~Application()
     AccountManager::instance()->shutdown();
 }
 
-void Application::slotAccountStateRemoved(AccountState *accountState)
+void Application::slotAccountStateRemoved(const AccountStatePtr &accountState)
 {
     if (_gui) {
-        disconnect(accountState, &AccountState::stateChanged,
+        disconnect(accountState.data(), &AccountState::stateChanged,
             _gui.data(), &ownCloudGui::slotAccountStateChanged);
         disconnect(accountState->account().data(), &Account::serverVersionChanged,
             _gui.data(), &ownCloudGui::slotTrayMessageIfServerUnsupported);
     }
     if (_folderManager) {
-        disconnect(accountState, &AccountState::stateChanged,
+        disconnect(accountState.data(), &AccountState::stateChanged,
             _folderManager.data(), &FolderMan::slotAccountStateChanged);
         disconnect(accountState->account().data(), &Account::serverVersionChanged,
             _folderManager.data(), &FolderMan::slotServerVersionChanged);

diff --git a/src/gui/application.h b/src/gui/application.h
@@ -98,7 +98,7 @@ protected slots:
     void slotUseMonoIconsChanged(bool);
     void slotCleanup();
     void slotAccountStateAdded(AccountState *accountState);
-    void slotAccountStateRemoved(AccountState *accountState);
+    void slotAccountStateRemoved(const AccountStatePtr &accountState);
     void slotSystemOnlineConfigurationChanged(QNetworkConfiguration);
 
 private:

diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp
@@ -913,13 +913,13 @@ void FolderMan::slotEtagPollTimerTimeout()
     }
 }
 
-void FolderMan::slotRemoveFoldersForAccount(AccountState *accountState)
+void FolderMan::slotRemoveFoldersForAccount(const AccountStatePtr &accountState)
 {
     QList<Folder *> foldersToRemove;
     // reserve a magic number
     foldersToRemove.reserve(16);
     for (auto *folder : qAsConst(_folderMap)) {
-        if (folder->accountState() == accountState) {
+        if (folder->accountState() == accountState.data()) {
             foldersToRemove.append(folder);
         }
     }

diff --git a/src/gui/folderman.h b/src/gui/folderman.h
@@ -302,7 +302,7 @@ private slots:
     void slotStartScheduledFolderSync();
     void slotEtagPollTimerTimeout();
 
-    void slotRemoveFoldersForAccount(AccountState *accountState);
+    void slotRemoveFoldersForAccount(const AccountStatePtr &accountState);
 
     // Wraps the Folder::syncStateChange() signal into the
     // FolderMan::folderSyncStateChange(Folder*) signal.

diff --git a/src/gui/models/activitylistmodel.cpp b/src/gui/models/activitylistmodel.cpp
@@ -269,9 +269,9 @@ void ActivityListModel::slotRefreshActivity(AccountState *ast)
     startFetchJob(ast);
 }
 
-void ActivityListModel::slotRemoveAccount(AccountState *ast)
+void ActivityListModel::slotRemoveAccount(const AccountStatePtr &ast)
 {
-    if (_activityLists.contains(ast)) {
+    if (_activityLists.contains(ast.data())) {
         const auto accountToRemove = ast->account()->uuid();
 
         QMutableListIterator<Activity> it(_finalList);
@@ -287,8 +287,8 @@ void ActivityListModel::slotRemoveAccount(AccountState *ast)
                 ++i;
             }
         }
-        _activityLists.remove(ast);
-        _currentlyFetching.remove(ast);
+        _activityLists.remove(ast.data());
+        _currentlyFetching.remove(ast.data());
     }
 }
 }
diff --git a/src/gui/models/activitylistmodel.h b/src/gui/models/activitylistmodel.h
@@ -17,6 +17,7 @@
 
 #include <QtCore>
 
+#include "accountstate.h"
 #include "activitydata.h"
 
 class QJsonDocument;
@@ -25,8 +26,6 @@ namespace OCC {
 
 Q_DECLARE_LOGGING_CATEGORY(lcActivity)
 
-class AccountState;
-
 /**
  * @brief The ActivityListModel
  * @ingroup gui
@@ -63,7 +62,7 @@ class ActivityListModel : public QAbstractTableModel
 
 public slots:
     void slotRefreshActivity(AccountState *ast);
-    void slotRemoveAccount(AccountState *ast);
+    void slotRemoveAccount(const AccountStatePtr &ast);
 
 signals:
     void activityJobStatusCode(AccountState *ast, int statusCode);

diff --git a/src/gui/settingsdialog.cpp b/src/gui/settingsdialog.cpp
@@ -423,14 +423,14 @@ void SettingsDialog::slotAccountDisplayNameChanged()
     }
 }
 
-void SettingsDialog::accountRemoved(AccountState *s)
+void SettingsDialog::accountRemoved(const AccountStatePtr &s)
 {
     for (auto it = _actionGroupWidgets.begin(); it != _actionGroupWidgets.end(); ++it) {
         auto as = qobject_cast<AccountSettings *>(*it);
         if (!as) {
             continue;
         }
-        if (as->accountsState() == s) {
+        if (as->accountsState() == s.data()) {
             _ui->toolBar->removeAction(it.key());
 
             if (_ui->stack->currentWidget() == it.value()) {

diff --git a/src/gui/settingsdialog.h b/src/gui/settingsdialog.h
@@ -73,7 +73,7 @@ public slots:
 
 private slots:
     void accountAdded(AccountState *);
-    void accountRemoved(AccountState *);
+    void accountRemoved(const AccountStatePtr &);
 
 private:
     void customizeStyle();

diff --git a/test/modeltests/testactivitymodel.cpp b/test/modeltests/testactivitymodel.cpp
@@ -40,7 +40,7 @@ private Q_SLOTS:
             Activity { Activity::ActivityType, 2, acc1, "test", "test", "foo.cpp", QUrl::fromUserInput("https://owncloud.com"), QDateTime::currentDateTime() },
             Activity { Activity::ActivityType, 4, acc2, "test", "test", "foo.cpp", QUrl::fromUserInput("https://owncloud.com"), QDateTime::currentDateTime() },
         });
-        model->slotRemoveAccount(AccountManager::instance()->accounts().first().data());
+        model->slotRemoveAccount(AccountManager::instance()->accounts().first());
     }
 };
 }