Lothario
/
axmol
mirror of https://github.com/axmolengine/axmol.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #6011 from DarraghCoy/event_listener_crash_fixes 

closed #4586: Fix crashes in event dispatch other related safety checks/fixes
This commit is contained in:
James Chen 2014-03-28 11:28:41 +08:00
parent 688ac51546 0f729126f0
commit 5c77b4d4b1
6 changed files with 266 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
cocos/2d/CCEventDispatcher.cpp
View File

@ -130,6 +130,16 @@ bool EventDispatcher::EventListenerVector::empty() const
void EventDispatcher::EventListenerVector::push_back(EventListener* listener)
{
#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS
CCASSERT(_sceneGraphListeners == nullptr ||
std::count(_sceneGraphListeners->begin(), _sceneGraphListeners->end(), listener) == 0,
"Listener should not be added twice!");
CCASSERT(_fixedListeners == nullptr ||
std::count(_fixedListeners->begin(), _fixedListeners->end(), listener) == 0,
"Listener should not be added twice!");
#endif
if (listener->getFixedPriority() == 0)
{
if (_sceneGraphListeners == nullptr)
@ -314,6 +324,11 @@ void EventDispatcher::resumeEventListenersForTarget(Node* target, bool recursive
void EventDispatcher::removeEventListenersForTarget(Node* target, bool recursive/* = false */)
{
// Ensure the node is removed from these immediately also.
// Don't want any dangling pointers or the possibility of dealing with deleted objects..
_nodePriorityMap.erase(target);
_dirtyNodes.erase(target);
auto listenerIter = _nodeListenersMap.find(target);
if (listenerIter != _nodeListenersMap.end())
{
@ -440,6 +455,69 @@ void EventDispatcher::addEventListenerWithSceneGraphPriority(EventListener* list
addEventListener(listener);
}
#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS && COCOS2D_DEBUG > 0
void EventDispatcher::debugCheckNodeHasNoEventListenersOnDestruction(Node* node)
{
// Check the listeners map
for (const auto & keyValuePair : _listenerMap)
{
const EventListenerVector * eventListenerVector = keyValuePair.second;
if (eventListenerVector)
{
if (eventListenerVector->getSceneGraphPriorityListeners())
{
for (EventListener * listener : *eventListenerVector->getSceneGraphPriorityListeners())
{
CCASSERT(!listener ||
listener->getSceneGraphPriority() != node,
"Node should have no event listeners registered for it upon destruction!");
}
}
}
}
// Check the node listeners map
for (const auto & keyValuePair : _nodeListenersMap)
{
CCASSERT(keyValuePair.first != node, "Node should have no event listeners registered for it upon destruction!");
if (keyValuePair.second)
{
for (EventListener * listener : *keyValuePair.second)
{
CCASSERT(listener->getSceneGraphPriority() != node,
"Node should have no event listeners registered for it upon destruction!");
}
}
}
// Check the node priority map
for (const auto & keyValuePair : _nodePriorityMap)
{
CCASSERT(keyValuePair.first != node,
"Node should have no event listeners registered for it upon destruction!");
}
// Check the to be added list
for (EventListener * listener : _toAddedListeners)
{
CCASSERT(listener->getSceneGraphPriority() != node,
"Node should have no event listeners registered for it upon destruction!");
}
// Check the dirty nodes set
for (Node * dirtyNode : _dirtyNodes)
{
CCASSERT(dirtyNode != node,
"Node should have no event listeners registered for it upon destruction!");
}
}
#endif // #if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS && COCOS2D_DEBUG > 0
void EventDispatcher::addEventListenerWithFixedPriority(EventListener* listener, int fixedPriority)
{
CCASSERT(listener, "Invalid parameters.");
@ -485,6 +563,7 @@ void EventDispatcher::removeEventListener(EventListener* listener)
if (l->getSceneGraphPriority() != nullptr)
{
dissociateNodeAndEventListener(l->getSceneGraphPriority(), l);
l->setSceneGraphPriority(nullptr); // NULL out the node pointer so we don't have any dangling pointers to destroyed nodes.
}
if (_inDispatch == 0)
@ -519,6 +598,18 @@ void EventDispatcher::removeEventListener(EventListener* listener)
setDirty(listener->getListenerID(), DirtyFlag::FIXED_PRIORITY);
}
}
#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS
CCASSERT(_inDispatch != 0 ||
!sceneGraphPriorityListeners ||
std::count(sceneGraphPriorityListeners->begin(), sceneGraphPriorityListeners->end(), listener) == 0,
"Listener should be in no lists after this is done if we're not currently in dispatch mode.");
CCASSERT(_inDispatch != 0 ||
!fixedPriorityListeners ||
std::count(fixedPriorityListeners->begin(), fixedPriorityListeners->end(), listener) == 0,
"Listener should be in no lists after this is done if we're not currently in dispatch mode.");
#endif
if (iter->second->empty())
{
@ -1127,6 +1218,7 @@ void EventDispatcher::removeEventListenersForListenerID(const EventListener::Lis
if (l->getSceneGraphPriority() != nullptr)
{
dissociateNodeAndEventListener(l->getSceneGraphPriority(), l);
l->setSceneGraphPriority(nullptr); // NULL out the node pointer so we don't have any dangling pointers to destroyed nodes.
}
if (_inDispatch == 0)

10
cocos/2d/CCEventDispatcher.h
View File

@ -140,6 +140,16 @@ public:
/** Destructor of EventDispatcher */
~EventDispatcher();
#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS && COCOS2D_DEBUG > 0
/**
* To help track down event listener issues in debug builds.
* Verifies that the node has no event listeners associated with it when destroyed.
*/
void debugCheckNodeHasNoEventListenersOnDestruction(Node* node);
#endif
protected:
friend class Node;

17
cocos/2d/CCNode.cpp
View File

@ -154,15 +154,15 @@ Node::~Node()
}
#endif
CC_SAFE_RELEASE(_actionManager);
CC_SAFE_RELEASE(_scheduler);
CC_SAFE_RELEASE_NULL(_actionManager);
CC_SAFE_RELEASE_NULL(_scheduler);
_eventDispatcher->removeEventListenersForTarget(this);
CC_SAFE_RELEASE(_eventDispatcher);
// attributes
CC_SAFE_RELEASE(_shaderProgram);
CC_SAFE_RELEASE(_userObject);
CC_SAFE_RELEASE_NULL(_shaderProgram);
CC_SAFE_RELEASE_NULL(_userObject);
for (auto& child : _children)
{
@ -175,7 +175,14 @@ Node::~Node()
#if CC_USE_PHYSICS
setPhysicsBody(nullptr);
#endif
#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS && COCOS2D_DEBUG > 0
_eventDispatcher->debugCheckNodeHasNoEventListenersOnDestruction(this);
#endif
CC_SAFE_RELEASE(_eventDispatcher);
}
bool Node::init()

11
cocos/2d/ccConfig.h
View File

@ -240,6 +240,17 @@ To enable set it to a value different than 0. Disabled by default.
#define CC_LABELATLAS_DEBUG_DRAW 0
#endif
/** @def CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS
If enabled (in conjunction with assertion macros) will verify on Node destruction that the node being destroyed has no event
listeners still associated with it in the event dispatcher. This can be used to track down problems where the event dispatch
system has dangling pointers to destroyed nodes.
Note: event listener verification will always be disabled in builds where assertions are disabled regardless of this setting.
*/
#ifndef CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS
#define CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS 0
#endif
/** @def CC_ENABLE_PROFILERS
If enabled, will activate various profilers within cocos2d. This statistical data will be output to the console
once per second showing average time (in milliseconds) required to execute the specific routine(s).

131
tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
View File

@ -26,7 +26,8 @@ std::function<Layer*()> createFunctions[] =
CL(StopPropagationTest),
CL(PauseResumeTargetTest),
CL(Issue4129),
CL(Issue4160)
CL(Issue4160),
CL(DanglingNodePointersTest)
};
unsigned int TEST_CASE_COUNT = sizeof(createFunctions) / sizeof(createFunctions[0]);
@ -1278,3 +1279,131 @@ std::string Issue4160::subtitle() const
{
return "Touch the red block twice \n should not crash and the red one couldn't be touched";
}
// DanglingNodePointersTest
class DanglingNodePointersTestSprite : public Sprite
{
public:
typedef std::function<void (DanglingNodePointersTestSprite * sprite)> TappedCallback;
static DanglingNodePointersTestSprite * create(const TappedCallback & tappedCallback)
{
auto ret = new DanglingNodePointersTestSprite(tappedCallback);
if (ret && ret->init())
{
ret->autorelease();
return ret;
}
CC_SAFE_DELETE(ret);
return nullptr;
}
protected:
DanglingNodePointersTestSprite(const TappedCallback & tappedCallback)
:
_eventListener(nullptr),
_tappedCallback(tappedCallback)
{
}
public:
void onEnter() override
{
Sprite::onEnter();
_eventListener = EventListenerTouchOneByOne::create();
_eventListener->setSwallowTouches(false);
_eventListener->onTouchBegan = [this](Touch* touch, Event* event) -> bool
{
_tappedCallback(this);
return false; // Don't claim the touch so it can propagate
};
_eventListener->onTouchEnded = [](Touch* touch, Event* event)
{
// Do nothing
};
_eventDispatcher->addEventListenerWithSceneGraphPriority(_eventListener, this);
}
void onExit() override
{
_eventDispatcher->removeEventListenersForTarget(this);
_eventListener = nullptr;
Sprite::onExit();
}
private:
EventListenerTouchOneByOne * _eventListener;
int _fixedPriority;
TappedCallback _tappedCallback;
};
DanglingNodePointersTest::DanglingNodePointersTest()
{
#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS == 1 && COCOS2D_DEBUG > 0
Point origin = Director::getInstance()->getVisibleOrigin();
Size size = Director::getInstance()->getVisibleSize();
auto callback2 = [](DanglingNodePointersTestSprite * sprite2)
{
CCASSERT(false, "This should never be called because the sprite gets removed from it's parent and destroyed!");
exit(1);
};
auto callback1 = [callback2, origin, size](DanglingNodePointersTestSprite * sprite1)
{
DanglingNodePointersTestSprite * sprite2 = dynamic_cast<DanglingNodePointersTestSprite*>(sprite1->getChildren().at(0));
CCASSERT(sprite2, "The first child of sprite 1 should be sprite 2!");
CCASSERT(sprite2->getReferenceCount() == 1, "There should only be 1 reference to sprite 1, from it's parent node. Hence removing it will destroy it!");
sprite1->removeAllChildren(); // This call should cause sprite 2 to be destroyed
// Recreate sprite 1 again
sprite2 = DanglingNodePointersTestSprite::create(callback2);
sprite2->setTexture("Images/MagentaSquare.png");
sprite2->setPosition(origin+Point(size.width/2, size.height/2));
sprite1->addChild(sprite2, -20);
};
auto sprite1 = DanglingNodePointersTestSprite::create(callback1); // Sprite 1 will receive touch before sprite 2
sprite1->setTexture("Images/CyanSquare.png");
sprite1->setPosition(origin+Point(size.width/2, size.height/2));
addChild(sprite1, -10);
auto sprite2 = DanglingNodePointersTestSprite::create(callback2); // Sprite 2 will be removed when sprite 1 is touched, should never receive an event.
sprite2->setTexture("Images/MagentaSquare.png");
sprite2->setPosition(origin+Point(size.width/2, size.height/2));
sprite1->addChild(sprite2, -20);
#endif
}
DanglingNodePointersTest::~DanglingNodePointersTest()
{
}
std::string DanglingNodePointersTest::title() const
{
return "DanglingNodePointersTest";
}
std::string DanglingNodePointersTest::subtitle() const
{
#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS == 1 && COCOS2D_DEBUG > 0
return "Tap the square - should not crash!";
#else
return "For test to work, must be compiled with:\n"
"CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS == 1\n&& COCOS2D_DEBUG > 0";
#endif
}

11
tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h
View File

@ -206,4 +206,15 @@ public:
private:
};
class DanglingNodePointersTest : public EventDispatcherTestDemo
{
public:
CREATE_FUNC(DanglingNodePointersTest);
DanglingNodePointersTest();
virtual ~DanglingNodePointersTest();
virtual std::string title() const override;
virtual std::string subtitle() const override;
};
#endif /* defined(__samples__NewEventDispatcherTest__) */