Exc access error

Question

Very new to C++ and having problems returning a vector. I put a breakpoint and the array is correct (populated with all the objects I would expect from the query). But when it returns I get an error: EXC_BAD_ACCESS on line m_pComponentContainer->removeAll(); from CCNode.cpp

Which is strange since this is a base class (does NOT inherit from any kind of CC object) although I am extensively using the Cocos2dx framework, its not included in this class.

Im fairly sure this is because something is being deallocated. However like I said Im very new to C++ and not really sure where the problem is. I was hoping to get a little further in development before I had to start worrying about memory management.

 int numberOfCards = DatabaseHelper::getNumberOfCards();

//cant be zero
assert(numberOfCards);

std::vector<CardSlot> returnArray(numberOfCards);

sqlite3_stmt * statement;

if (sqlite3_open(this->dbpath.c_str(),&this->cardWarsDB) == SQLITE_OK)
{
    const char* query_stmt = "select ID, HP, MP, AbilityText from Cards WHERE ID IN (SELECT DISTINCT cardsID FROM Deck WHERE name = 'All')";

    if (sqlite3_prepare_v2(this->cardWarsDB, query_stmt, -1, &statement, NULL) == SQLITE_OK)
    {
        while (sqlite3_step(statement) == SQLITE_ROW)
        {
            CardSlot *aCard;

            const char* cardID = (const char*)sqlite3_column_text(statement, 0);
            const char* cardHP = (const char*)sqlite3_column_text(statement, 1);
            const char* cardMP = (const char*)sqlite3_column_text(statement, 2);
            const char* cardAbility = (const char*)sqlite3_column_text(statement, 3);

            if (cardID != NULL) {
                std::string imageName = ".png";
                imageName = cardID + imageName;
                aCard = (CardSlot *)CardSlot::spriteWithFile(imageName.c_str());
            }

            if (cardID != NULL) {
                aCard->cardID = std::string(cardID);
                cocos2d::CCLog("DB returned results, cardID: %s",aCard->cardID.c_str());
            }
            if (cardHP != NULL) {
                aCard->cardHP = std::string(cardHP);
                cocos2d::CCLog("DB returned results, cardHP: %s",aCard->cardHP.c_str());
            }
            if (cardMP != NULL) {
                aCard->cardMP = std::string(cardMP);
                cocos2d::CCLog("DB returned results, cardMP: %s",aCard->cardMP.c_str());
            }
            if (cardAbility != NULL) {
                aCard->cardAbility = std::string(cardAbility);
                cocos2d::CCLog("DB returned results, cardAbility: %s",aCard->cardAbility.c_str());
            }

            numberOfCards--;

            returnArray[numberOfCards] = *aCard;

        }
        sqlite3_finalize(statement);
    }
    sqlite3_close(this->cardWarsDB);
    return returnArray;
}

Here is a screenshot of the stack trace. I was just looking at it, and it seems that it is the CardSlot objects are the culprits. But still dont know how to "retain" them, but Ill look at some Cocos documentation.

enter image description here

NOTE1

Perhaps sqlite_step iterates more than numberOfCards times? You could put a simple check in there, such as: `if(numberOfCards-- == 0) reportError();` — SeaBass, Aug 04 '13 at 23:14
nope :( it runs five times. it hits zero but thats by design (for the 0 index) — owen gerig, Aug 05 '13 at 01:24
Okay, but are you checking the value before or after the increment? If the condition `(numberOfCards-- == 0)` is true then you've got an access violation. — SeaBass, Aug 05 '13 at 01:37
Your stack trace shows the destructor of `CardSlot`. This is what needs your attention, along with the copy constructor and the assignment operator. — n. m. could be an AI, Aug 05 '13 at 05:31
This line: `aCard = * CardSlot::spriteWithFile(imageName.c_str());`, seems very, very suspicious. — nneonneo, Aug 05 '13 at 07:59

score 1 · Answer 1 · answered Aug 05 '13 at 08:01

1

It looks like your CardSlot is not safe to copy. You copy CardSlots in at least two places:

aCard = * CardSlot::spriteWithFile(imageName.c_str()); (also a memory leak assuming spriteWithFile returns CardSlot *; the "temporary" is not destructed)
returnArray[numberOfCards] = aCard;

From what I can tell, you are probably keeping a CCSprite pointer in CardSlot and destroying it (with delete) in your CardSlot destructor. However, this pointer gets destroyed multiple times because of the copies, which causes your crash.

You need to redesign your class so it can either be safely copied, or refactor your code so that you make no copies (e.g. by using a vector<shared_ptr<CardSlot> > to hold pointers to the instances).

answered Aug 05 '13 at 08:01

nneonneo

171,345
36
312
383

I have been investigating what you said, couple of questions. What is the preferred method? It seems to me like a vector of pointers is fine but should I be concerned that my class cannot be copied? I do not have any destructors though I suppose CCNode (Parent to CardSlot) does and thats whats being called. what confuses me is why its being deconstructed if it is still be referenced by the vector. Also I made aCard a pointer instead of an instantiated object that should get rid of the leak correct (check edits above)? – owen gerig Aug 05 '13 at 17:21
1

@owengerig: C++ doesn't do reference counting. Each instance of the `CardSlot` is separate: writing `CardSlot a,b; a = b;` *copies* `b` to `a` entirely. – nneonneo Aug 05 '13 at 22:40
No, I know theres no referencing counters in C++ but most of the terminology I use comes from obj-c :( . If we revisit "what is the preferred method" it would seem to me that I dont want to be creating copies and should be using pointers instead. – owen gerig Aug 07 '13 at 17:04

score 0 · Accepted Answer · answered Aug 07 '13 at 16:59

I have edited the code to use more pointers rather then passing around and filling my array with objects. However I think a big thing that helped fix it was using cocos2d::CCArray instead of a std::vector. Most of my classes are children of Cocos2d classes (CCSprites, and CCLayers) and so using its own array data type makes sense.

cocos2d::CCArray DatabaseHelper::getAllCards()
{
    int numberOfCards = DatabaseHelper::getNumberOfCards();

    //cant be zero
    assert(numberOfCards);

    cocos2d::CCArray returnArray(numberOfCards);

    sqlite3_stmt * statement;

    if (sqlite3_open(this->dbpath.c_str(),&this->cardWarsDB) == SQLITE_OK)
    {
        const char* query_stmt = "select ID, HP, MP, AbilityText from Cards WHERE ID IN (SELECT DISTINCT cardsID FROM Deck WHERE name = 'All')";

        if (sqlite3_prepare_v2(this->cardWarsDB, query_stmt, -1, &statement, NULL) == SQLITE_OK)
        {
            while (sqlite3_step(statement) == SQLITE_ROW)
            {
                CardSlot* aCard;

                const char* cardID = (const char*)sqlite3_column_text(statement, 0);
                const char* cardHP = (const char*)sqlite3_column_text(statement, 1);
                const char* cardMP = (const char*)sqlite3_column_text(statement, 2);
                const char* cardAbility = (const char*)sqlite3_column_text(statement, 3);

                if (cardID != NULL) {
                    std::string imageName = ".png";
                    imageName = cardID + imageName;
                    aCard = CardSlot::spriteWithFile(imageName.c_str());
                }

                if (cardID != NULL) {
                    aCard->cardID = std::string(cardID);
                    cocos2d::CCLog("DB returned results, cardID: %s",aCard->cardID.c_str());
                }
                if (cardHP != NULL) {
                    aCard->cardHP = std::string(cardHP);
                    cocos2d::CCLog("DB returned results, cardHP: %s",aCard->cardHP.c_str());
                }
                if (cardMP != NULL) {
                    aCard->cardMP = std::string(cardMP);
                    cocos2d::CCLog("DB returned results, cardMP: %s",aCard->cardMP.c_str());
                }
                if (cardAbility != NULL) {
                    aCard->cardAbility = std::string(cardAbility);
                    cocos2d::CCLog("DB returned results, cardAbility: %s",aCard->cardAbility.c_str());
                }

                numberOfCards--;

                returnArray.addObject(aCard);

            }
            sqlite3_finalize(statement);
        }
        sqlite3_close(this->cardWarsDB);
        return returnArray;
    }

    //incase sql fails, close db and created a "FAILED" card
    sqlite3_close(this->cardWarsDB);
    cocos2d::CCLog("DB returned error: cant open char catagories file");
    cocos2d::CCArray failedReturnArray(1);
    CardSlot * aCard;
    aCard->cardID = std::string("FAILED");
    aCard->cardHP = std::string("FAILED");
    aCard->cardMP = std::string("FAILED");
    aCard->cardAbility = std::string("FAILED");
    failedReturnArray.addObject(aCard);
    return failedReturnArray;
}

Also in case anyone cares here is CardSlot (not much to it, only built the constructor at this time):

CardSlot * CardSlot::spriteWithFile(const char *pszFileName)
{

    CCLOG("CardSlot::spriteWithFile");
    CardSlot * aCard = new CardSlot();
    if (aCard && aCard->initWithFile(pszFileName))
    {
        aCard->cardID = pszFileName;
        aCard->scheduleUpdate();
        aCard->autorelease();
        return aCard;
    }
    CC_SAFE_DELETE(aCard);
    return NULL;
}

The only thing Im concerned about is that I think my CCArray should be a pointer. But its working now and learning all the memory management "tricks of the trade" will come in time, the more I work with C++

Thanks @nneonneo for all the help Im sure your fix would have worked and I tried but no matter what I did couldnt get the vector to work. I 1up'd you as much as I could but really this is the "Answer" I implemented.

score -1 · Answer 3 · answered Aug 05 '13 at 07:49

-1

The returnArray is declared local to the function, it will be deallocated when the function returns. You would need to either declare it as static or move the declaration to outside the function.

answered Aug 05 '13 at 07:49

Edward Clements

5,040
2
21
27

2

It is perfectly legal to return a `vector` object by value. – CL. Aug 05 '13 at 08:01

Exc access error

3 Answers3