jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-11-05 01:23:24 -08:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

[CardDB] Clear CardInfo relations before deferred deletion (#7615)

Browse source 
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 <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL 2026-10-24 10:28:24 +02:00 • committed by GitHub
parent 6e770eb7b2
commit 4028606bd9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 49 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

7
libcockatrice_card/libcockatrice/card/card_info.cpp
View file

@ -270,6 +270,13 @@ void CardInfo::resetReverseRelatedCards2Me()
reverseRelatedCardsToMe = QList<CardRelation *>();
}
void CardInfo::clearDirectRelations()
{
relatedCards = QList<CardRelation *>();
reverseRelatedCards = QList<CardRelation *>();
reverseRelatedCardsToMe = QList<CardRelation *>();
}
// Back-compatibility methods. Remove ASAP
QString CardInfo::getCardType() const
{

8
libcockatrice_card/libcockatrice/card/card_info.h
View file

@ -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);

7
libcockatrice_card/libcockatrice/card/database/card_database.cpp
View file

@ -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);

27
tests/carddatabase/carddatabase_test.cpp
View file

@ -26,6 +26,7 @@
#include <libcockatrice/card/database/parser/cockatrice_xml_4.h>
#include <libcockatrice/card/lazy_properties_hash.h>
#include <libcockatrice/card/printing/printing_info.h>
#include <libcockatrice/card/relation/card_relation.h>
#include <libcockatrice/interfaces/interface_card_database_path_provider.h>
#include <libcockatrice/interfaces/interface_card_set_priority_controller.h>
#include <libcockatrice/interfaces/noop_card_preference_provider.h>
@ -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<QString, QString>(), {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;