Fix memory leaks from DeckLoader usage (#5665)

* add comment

* stack allocate DeckLoader for loading tags

* deckModel now takes ownership of DeckLoader

* fix remaining

* add comment

cockatrice/src/client/tabs/abstract_tab_deck_editor.cpp

@@ -128,6 +128,10 @@ void AbstractTabDeckEditor::actSwapCard(CardInfoPtr info, QString zoneName)
         deckDockWidget->deckModel->findCard(info->getName(), zoneName, providerId, collectorNumber));
 }
 
+/**
+ * Sets the currently active deck for this tab
+ * @param _deck The deck. Takes ownership of the object
+ */
 void AbstractTabDeckEditor::setDeck(DeckLoader *_deck)
 {
     deckDockWidget->setDeck(_deck);
@@ -289,12 +293,13 @@ void AbstractTabDeckEditor::openDeckFromFile(const QString &fileName, DeckOpenLo
         SettingsCache::instance().recents().updateRecentlyOpenedDeckPaths(fileName);
         if (deckOpenLocation == NEW_TAB) {
             emit openDeckEditor(l);
+            l->deleteLater();
         } else {
             deckMenu->setSaveStatus(false);
             setDeck(l);
         }
     } else {
-        delete l;
+        l->deleteLater();
         QMessageBox::critical(this, tr("Error"), tr("Could not open deck at %1").arg(fileName));
     }
     deckMenu->setSaveStatus(true);

cockatrice/src/client/tabs/tab_supervisor.cpp

@@ -764,6 +764,10 @@ void TabSupervisor::talkLeft(TabMessage *tab)
     removeTab(indexOf(tab));
 }
 
+/**
+ * Creates a new deck editor tab
+ * @param deckToOpen The deck to open in the tab. Creates a copy of the DeckLoader instance.
+ */
 TabDeckEditor *TabSupervisor::addDeckEditorTab(const DeckLoader *deckToOpen)
 {
     auto *tab = new TabDeckEditor(this);

cockatrice/src/client/ui/widgets/deck_editor/deck_editor_deck_dock_widget.cpp

@@ -289,6 +289,10 @@ void DeckEditorDeckDockWidget::setBannerCard(int /* changedIndex */)
     emit deckChanged();
 }
 
+/**
+ * Sets the currently active deck for this tab
+ * @param _deck The deck. Takes ownership of the object
+ */
 void DeckEditorDeckDockWidget::setDeck(DeckLoader *_deck)
 {
     deckModel->setDeckList(_deck);

cockatrice/src/client/ui/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp

@@ -143,10 +143,10 @@ void DeckPreviewDeckTagsDisplayWidget::openTagEditDlg()
             auto *deckEditor = qobject_cast<TabDeckEditor *>(currentParent);
             QStringList knownTags;
             QStringList allFiles = getAllFiles(SettingsCache::instance().getDeckPath());
-            auto *loader = new DeckLoader();
+            DeckLoader loader;
             for (const QString &file : allFiles) {
-                loader->loadFromFile(file, DeckLoader::getFormatFromName(file), false);
-                QStringList tags = loader->getTags();
+                loader.loadFromFile(file, DeckLoader::getFormatFromName(file), false);
+                QStringList tags = loader.getTags();
                 knownTags.append(tags);
                 knownTags.removeDuplicates();
             }

cockatrice/src/deck/deck_list_model.cpp

@@ -18,6 +18,7 @@ DeckListModel::DeckListModel(QObject *parent)
     : QAbstractItemModel(parent), lastKnownColumn(1), lastKnownOrder(Qt::AscendingOrder)
 {
     deckList = new DeckLoader;
+    deckList->setParent(this);
     connect(deckList, &DeckLoader::deckLoaded, this, &DeckListModel::rebuildTree);
     connect(deckList, &DeckLoader::deckHashChanged, this, &DeckListModel::deckHashChanged);
     root = new InnerDecklistNode;
@@ -26,7 +27,6 @@ DeckListModel::DeckListModel(QObject *parent)
 DeckListModel::~DeckListModel()
 {
     delete root;
-    delete deckList;
 }
 
 void DeckListModel::rebuildTree()
@@ -470,10 +470,14 @@ void DeckListModel::cleanList()
     setDeckList(new DeckLoader);
 }
 
+/**
+ * @param _deck The deck. Takes ownership of the object
+ */
 void DeckListModel::setDeckList(DeckLoader *_deck)
 {
-    delete deckList;
+    deckList->deleteLater();
     deckList = _deck;
+    deckList->setParent(this);
     connect(deckList, &DeckLoader::deckLoaded, this, &DeckListModel::rebuildTree);
     connect(deckList, &DeckLoader::deckHashChanged, this, &DeckListModel::deckHashChanged);
     rebuildTree();

cockatrice/src/dialogs/dlg_load_deck_from_clipboard.cpp

@@ -63,13 +63,14 @@ void DlgLoadDeckFromClipboard::actOK()
     QTextStream stream(&buffer);
 
     auto *deckLoader = new DeckLoader;
+    deckLoader->setParent(this);
+
     if (buffer.contains("<cockatrice_deck version=\"1\">")) {
         if (deckLoader->loadFromString_Native(buffer)) {
             deckList = deckLoader;
             accept();
         } else {
             QMessageBox::critical(this, tr("Error"), tr("Invalid deck list."));
-            delete deckLoader;
         }
     } else if (deckLoader->loadFromStream_Plain(stream)) {
         deckList = deckLoader;
@@ -81,7 +82,6 @@ void DlgLoadDeckFromClipboard::actOK()
         accept();
     } else {
         QMessageBox::critical(this, tr("Error"), tr("Invalid deck list."));
-        delete deckLoader;
     }
 }
 

cockatrice/src/dialogs/dlg_load_deck_from_clipboard.h

@@ -24,6 +24,14 @@ private:
 
 public:
     explicit DlgLoadDeckFromClipboard(QWidget *parent = nullptr);
+
+    /**
+     * Gets the loaded deck. Only call this method after this dialog window has been successfully exec'd.
+     *
+     * The returned DeckLoader is parented to this object; make sure to take ownership of the DeckLoader if you intend
+     * to use it, since otherwise it will get destroyed once this dlg is destroyed
+     * @return The DeckLoader
+     */
     DeckLoader *getDeckList() const
     {
         return deckList;