From 4028606bd9128058a6ab2e957abe966d5778634f Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:28:24 +0200 Subject: [PATCH] [CardDB] Clear CardInfo relations before deferred deletion (#7615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit removeCard() and swapInDatabaseData() schedule CardRelation deletion with deleteLater(), but the owning CardInfo could outlive them through an external shared pointer and kept returning the freed pointers from getAllRelatedCards(). Add CardInfo::clearDirectRelations() and call it after scheduling deletion so the UI can no longer dereference dangling relations. Co-authored-by: Lukas BrĂ¼bach --- .../libcockatrice/card/card_info.cpp | 7 +++++ .../libcockatrice/card/card_info.h | 8 ++++++ .../card/database/card_database.cpp | 7 +++++ tests/carddatabase/carddatabase_test.cpp | 27 +++++++++++++++++++ 4 files changed, 49 insertions(+) diff --git a/libcockatrice_card/libcockatrice/card/card_info.cpp b/libcockatrice_card/libcockatrice/card/card_info.cpp index c90027442..388c9ab57 100644 --- a/libcockatrice_card/libcockatrice/card/card_info.cpp +++ b/libcockatrice_card/libcockatrice/card/card_info.cpp @@ -270,6 +270,13 @@ void CardInfo::resetReverseRelatedCards2Me() reverseRelatedCardsToMe = QList(); } +void CardInfo::clearDirectRelations() +{ + relatedCards = QList(); + reverseRelatedCards = QList(); + reverseRelatedCardsToMe = QList(); +} + // Back-compatibility methods. Remove ASAP QString CardInfo::getCardType() const { diff --git a/libcockatrice_card/libcockatrice/card/card_info.h b/libcockatrice_card/libcockatrice/card/card_info.h index 77ddce5ce..eb1250a1c 100644 --- a/libcockatrice_card/libcockatrice/card/card_info.h +++ b/libcockatrice_card/libcockatrice/card/card_info.h @@ -453,6 +453,14 @@ public: return result; } void resetReverseRelatedCards2Me(); + /** + * @brief Clears the raw relation lists without deleting the CardRelation objects. + * + * Callers that schedule the relations for deletion (e.g. CardDatabase::removeCard()) + * must use this to drop the now-dangling pointers from this CardInfo, which may + * outlive them through external shared pointers. + */ + void clearDirectRelations(); void addReverseRelatedCards2Me(CardRelation *cardRelation) { reverseRelatedCardsToMe.append(cardRelation); diff --git a/libcockatrice_card/libcockatrice/card/database/card_database.cpp b/libcockatrice_card/libcockatrice/card/database/card_database.cpp index 7b0d5b2f9..946412606 100644 --- a/libcockatrice_card/libcockatrice/card/database/card_database.cpp +++ b/libcockatrice_card/libcockatrice/card/database/card_database.cpp @@ -168,6 +168,10 @@ void CardDatabase::removeCard(const CardInfoPtr &card) cardRelation->deleteLater(); } + // The relations are only scheduled for deletion; the CardInfo may outlive + // them through an external shared pointer, so drop the raw pointers now. + card->clearDirectRelations(); + QMutexLocker locker(removeCardMutex); cards.remove(card->getName()); simpleNameCards.remove(card->getSimpleName()); @@ -271,6 +275,9 @@ void CardDatabase::swapInDatabaseData(CardDatabaseData data) for (auto *rel : card->getReverseRelatedCards2Me()) { rel->deleteLater(); } + // Replacement cards take over the database; old CardInfo objects may + // still be referenced by the UI, so clear their dangling relations. + card->clearDirectRelations(); } cards = std::move(data.cards); diff --git a/tests/carddatabase/carddatabase_test.cpp b/tests/carddatabase/carddatabase_test.cpp index 1b68b8166..eb522434d 100644 --- a/tests/carddatabase/carddatabase_test.cpp +++ b/tests/carddatabase/carddatabase_test.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -124,6 +125,32 @@ TEST(CardDatabaseTest, FindPrintingWithIdOnNullCardInfoReturnsEmpty) EXPECT_TRUE(db.query()->findPrintingWithId(nullptr, "card_Not a Card_fixture").isEmpty()); } +TEST(CardInfoTest, ClearDirectRelationsDropsRawPointers) +{ + auto *forward = new CardRelation("Child"); + auto *reverse = new CardRelation("Parent"); + auto *toMe = new CardRelation("Child"); + + auto card = CardInfo::newInstance("Parent", "", false, QHash(), {forward}, {reverse}, + SetToPrintingsMap(), CardInfo::UiAttributes()); + card->addReverseRelatedCards2Me(toMe); + ASSERT_EQ(1, card->getRelatedCards().size()); + ASSERT_EQ(1, card->getReverseRelatedCards().size()); + ASSERT_EQ(1, card->getReverseRelatedCards2Me().size()); + ASSERT_EQ(2, card->getAllRelatedCards().size()); + + card->clearDirectRelations(); + + EXPECT_TRUE(card->getRelatedCards().isEmpty()); + EXPECT_TRUE(card->getReverseRelatedCards().isEmpty()); + EXPECT_TRUE(card->getReverseRelatedCards2Me().isEmpty()); + EXPECT_TRUE(card->getAllRelatedCards().isEmpty()); + + delete forward; + delete reverse; + delete toMe; +} + TEST(CardDatabaseTest, Xml4LocalizedDataRoundTrip) { NoopCardSetPriorityController controller;