Crash in CCNotificationCenter

If you remove an observer within a response to the same notification you will get a crash, as the CCNotificationObserver will get destroyed at the same time its getting a response.

An easy fix is to retain the item before removing it from the array, and then autoreleasing it.

void CCNotificationCenter::removeObserver(CCObject *target,const char *name)
{
    CCObject* obj = NULL;
    CCARRAY_FOREACH(m_observers, obj)
    {
        CCNotificationObserver* observer = (CCNotificationObserver*) obj;
        if (!observer)
            continue;

        if (!strcmp(observer->getName(),name) && observer->getTarget() == target)
        {
            observer->retain(); //FIX 1: retain item before removing
            m_observers->removeObject(observer);
            observer->autorelease(); // FIX 2: then autorelease
            return;
        }
    }
}

Hi, I couldn’t reproduce this issue by modifying the NotificationTest as follows:

// Light::switchStateChanged is the callback function, I removed the observer in this function, but did not get a crash.
void Light::switchStateChanged(CCObject* obj)
{

    CCNotificationCenter::sharedNotifCenter()->removeObserver(this, MSG_SWITCH_STATE);
    int index = (int)obj;
    s_bSwitchOn = index == 0 ? false : true;
    updateLightState();
}

Could you provide more codes for us to reproduce this bug?

I meet this problem. Using this fix, it works.

EJ Mablekos wrote:

If you remove an observer within a response to the same notification you will get a crash, as the CCNotificationObserver will get destroyed at the same time its getting a response.
>
An easy fix is to retain the item before removing it from the array, and then autoreleasing it.
>
>
[…]

@土豆 薛
I ran the test suggested by James, it worked ok on iOS.
Could you paste some codes to reproduce it?
And what’s your cocos2d-x version?
What’s the platform you tested on?

project is too huge. i am not sure what cause this problem.

cocos2d-x version is : cocos2d-2.0-rc2-x-2.0.1 @ Jun 29 2012
test on iPhone 5.1 simulator.

Minggo Zhang wrote:

@土豆 薛
I ran the test suggested by James, it worked ok on iOS.
Could you paste some codes to reproduce it?
And what’s your cocos2d-x version?
What’s the platform you tested on?

I ran into this as well, and the fix above did work. I had it on iOS (as well as Android), but debugged in IOS. The problem was that in a call back I removed two observers, which was ok, but when the ‘post notification’ loop continued, the observers was 2 less than before (59 to 57), so once it hit object #59, it got a bad EXEC as that had been removed from the array. The root cause is simply we’re removing objects in the array while it’s still being looped through, it doesn’t make sense to break the loop at the match because it’s very likely you’ll have multiple listeners for the same notification. So the fix above looks good, but maybe the snippet below can help reproduce for the leads.

Here’s something to reproduce, (The SFDialog class is a popup dialog with a CCMenu, and all it does is send a notification when the button is pressed (in this case kSFDialogCancelButton).

void MainGame::promptForNewGame()
{

    SFDialog *db = SFDialog::dialogWithTitle("Play Again?", "Would you like to start another game?", "Cancel", "New Game");

    CCNotificationCenter::sharedNotificationCenter()->addObserver(this, callfuncO_selector(GameBlitz::SFDialogCallback),
                                                                  kSFDialogCancelButton, NULL);

    CCNotificationCenter::sharedNotificationCenter()->addObserver(this, callfuncO_selector(GameBlitz::prepareGame),
                                                                  kSFDialogOKButton, NULL);

    this->addChild(db, 1000, kDialogTag);
}

void MainGame::SFDialogCallback(CCObject *obj)
{
    CCNotificationCenter::sharedNotificationCenter()->removeObserver(this, kSFDialogCancelButton);       
    CCNotificationCenter::sharedNotificationCenter()->removeObserver(this, kSFDialogOKButton);    

    this->removeChildByTag(kDialogTag, true);
}