From 0ee1095e37d53408cb674eb3e2659e5d2cd36e39 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Wed, 26 Mar 2014 13:12:44 -0700
Subject: [PATCH 01/12] Fix crashes in event dispatch other related safety
 checks/fixes

1: Add a fix to prevent events from being sent to scene nodes which are destroyed. Previously when a node was being destroyed it would try to unregister any event listeners it would have. This process would fail however in the case where event dispatching was already underway because we can't modify the listeners list while we are iterating through it. This is a pretty common occurrence since we often tear down & switch scenes in response to button clicks etc. Fix the problem by nulling out the slot for the listener we are removing in this case, so that it's node no longer receives any events after it is destroyed. The event dispatching code will cleanup this unused/nulled slot once the event processing loop has terminated.

2: When removing an event listener in cocos2d::EventDispatcher, ensure immediately it gets removed from the node priority map and dirty nodes set in order to ensure we don't inadvertently access any dangling pointers to nodes.

3: Add debug checks to ensure nodes have no associated event listeners after they are destroyed. This check will catch the problem fixed by (1) if it is still present.

4: Add some extra debug sanity checks in the event dispatcher to ensure nodes are not added to the lists twice and that they are being properly removed from the lists under certain conditions.

5: In cocos2d::Node's destructor NULL out members after releasing them in case they somehow happen to be accessed during the destructor. Move the release of the event dispatcher to the very end, since we need to check against it in debug.
---
 cocos/2d/CCEventDispatcher.cpp | 134 ++++++++++++++++++++++++++++++++-
 cocos/2d/CCEventDispatcher.h   |  10 +++
 cocos/2d/CCNode.cpp            |  17 +++--
 3 files changed, 153 insertions(+), 8 deletions(-)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index 6b12aa58e1..a5149be02f 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -130,6 +130,14 @@ bool EventDispatcher::EventListenerVector::empty() const
 
 void EventDispatcher::EventListenerVector::push_back(EventListener* listener)
 {
+    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!");
+
     if (listener->getFixedPriority() == 0)
     {
         if (_sceneGraphListeners == nullptr)
@@ -314,6 +322,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 +453,79 @@ void EventDispatcher::addEventListenerWithSceneGraphPriority(EventListener* list
     addEventListener(listener);
 }
 
+#if 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->getFixedPriorityListeners())
+            {
+                for (EventListener * listener : *eventListenerVector->getFixedPriorityListeners())
+                {
+                    CCASSERT(!listener ||
+                             listener->getSceneGraphPriority() != node,
+                             "Node should have no event listeners registered for it upon destruction!");
+                }
+            }
+            
+            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 COCOS2D_DEBUG > 0
+
+
 void EventDispatcher::addEventListenerWithFixedPriority(EventListener* listener, int fixedPriority)
 {
     CCASSERT(listener, "Invalid parameters.");
@@ -492,7 +578,14 @@ void EventDispatcher::removeEventListener(EventListener* listener)
                     listeners->erase(iter);
                     CC_SAFE_RELEASE(l);
                 }
-                
+                else
+                {
+                	// Fix to prevent events propagating to nodes or listeners that are destroyed:
+                    // Since we can't remove the listener while we're iterating the listeners list,
+                    // null out the pointer. We'll erase the actual pointer storage later.
+                    *iter = nullptr;
+                }
+
                 isFound = true;
                 break;
             }
@@ -519,6 +612,16 @@ void EventDispatcher::removeEventListener(EventListener* listener)
                 setDirty(listener->getListenerID(), DirtyFlag::FIXED_PRIORITY);
             }
         }
+        
+        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.");			 
 
         if (iter->second->empty())
         {
@@ -613,6 +716,11 @@ void EventDispatcher::dispatchEventToListeners(EventListenerVector* listeners, c
             // priority == 0, scene graph priority
             for (auto& l : *sceneGraphPriorityListeners)
             {
+                if (!l)
+                {
+                    continue;   // Event listener slot which was temporarily nulled because we removed the listener while dispatching events.
+                }
+
                 if (!l->isPaused() && l->isRegistered() && onEvent(l))
                 {
                     shouldStopPropagation = true;
@@ -900,7 +1008,13 @@ void EventDispatcher::updateListeners(Event* event)
             for (auto iter = sceneGraphPriorityListeners->begin(); iter != sceneGraphPriorityListeners->end();)
             {
                 auto l = *iter;
-                if (!l->isRegistered())
+
+                if (!l)
+                {
+                    // Event listener slot which was temporarily nulled because we removed the listener while dispatching events.
+                    iter = sceneGraphPriorityListeners->erase(iter);
+                }
+                else if (!l->isRegistered())
                 {
                     iter = sceneGraphPriorityListeners->erase(iter);
                     l->release();
@@ -917,7 +1031,13 @@ void EventDispatcher::updateListeners(Event* event)
             for (auto iter = fixedPriorityListeners->begin(); iter != fixedPriorityListeners->end();)
             {
                 auto l = *iter;
-                if (!l->isRegistered())
+
+                if (!l)
+                {
+                    // Event listener slot which was temporarily nulled because we removed the listener while dispatching events.
+                    iter = fixedPriorityListeners->erase(iter);
+                }
+                else if (!l->isRegistered())
                 {
                     iter = fixedPriorityListeners->erase(iter);
                     l->release();
@@ -1123,6 +1243,14 @@ void EventDispatcher::removeEventListenersForListenerID(const EventListener::Lis
             for (auto iter = listenerVector->begin(); iter != listenerVector->end();)
             {
                 auto l = *iter;
+
+	            if (!l)
+	            {
+	                // Listener slot which was erased by nulling out the slot.
+	                // This happens when we try to remove a listener while dispatching events.
+	                continue;
+	            }
+				
                 l->setRegistered(false);
                 if (l->getSceneGraphPriority() != nullptr)
                 {
diff --git a/cocos/2d/CCEventDispatcher.h b/cocos/2d/CCEventDispatcher.h
index 1178cb8444..0628e58bea 100644
--- a/cocos/2d/CCEventDispatcher.h
+++ b/cocos/2d/CCEventDispatcher.h
@@ -140,6 +140,16 @@ public:
     /** Destructor of EventDispatcher */
     ~EventDispatcher();
 
+#if 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;
     
diff --git a/cocos/2d/CCNode.cpp b/cocos/2d/CCNode.cpp
index b086275606..4b06df0edf 100644
--- a/cocos/2d/CCNode.cpp
+++ b/cocos/2d/CCNode.cpp
@@ -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 COCOS2D_DEBUG > 0
+		_eventDispatcher->debugCheckNodeHasNoEventListenersOnDestruction(this);
+	#endif
+
+    CC_SAFE_RELEASE(_eventDispatcher);
 }
 
 bool Node::init()

From 07646f1c827d289869898fb7f4e5ca7c60d1e711 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 11:28:27 -0700
Subject: [PATCH 02/12] Review feedback: fix memory leak discovered. Always
 safe release the listener we retain - shift where release happens to the end
 of the block. Reproduced the leak using the Xcode Leaks instrument in the
 RemoveListenerAfterAddingTest and verified fixed after apply this change.

---
 cocos/2d/CCEventDispatcher.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index a5149be02f..159112e374 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -576,7 +576,6 @@ void EventDispatcher::removeEventListener(EventListener* listener)
                 if (_inDispatch == 0)
                 {
                     listeners->erase(iter);
-                    CC_SAFE_RELEASE(l);
                 }
                 else
                 {
@@ -587,6 +586,7 @@ void EventDispatcher::removeEventListener(EventListener* listener)
                 }
 
                 isFound = true;
+                CC_SAFE_RELEASE(l);
                 break;
             }
         }

From 6e19da12c308814d2fed19a72f874cf356f78938 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 11:59:07 -0700
Subject: [PATCH 03/12] Review feedback: Remove unnecessary check of
 EventListener::getSceneGraphPriority() for fixed event listeners. Fixed
 priority event listeners never have an associated node.

---
 cocos/2d/CCEventDispatcher.cpp | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index 159112e374..d10ef8db09 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -464,16 +464,6 @@ void EventDispatcher::debugCheckNodeHasNoEventListenersOnDestruction(Node* node)
         
         if (eventListenerVector)
         {
-            if (eventListenerVector->getFixedPriorityListeners())
-            {
-                for (EventListener * listener : *eventListenerVector->getFixedPriorityListeners())
-                {
-                    CCASSERT(!listener ||
-                             listener->getSceneGraphPriority() != node,
-                             "Node should have no event listeners registered for it upon destruction!");
-                }
-            }
-            
             if (eventListenerVector->getSceneGraphPriorityListeners())
             {
                 for (EventListener * listener : *eventListenerVector->getSceneGraphPriorityListeners())

From 8ee90389b4909861a936aa75bcdb4897efa8b101 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 12:29:28 -0700
Subject: [PATCH 04/12] Review feedback: Wrap the debug verification in
 additional macros which are disabled by default. Require the user to enable
 'CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS' in ccConfig.h in order to enable the
 debug checks in debug builds.

---
 cocos/2d/CCEventDispatcher.cpp |  4 ++--
 cocos/2d/CCEventDispatcher.h   |  2 +-
 cocos/2d/CCNode.cpp            |  6 +++---
 cocos/2d/ccConfig.h            | 11 +++++++++++
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index d10ef8db09..2d146cce27 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -453,7 +453,7 @@ void EventDispatcher::addEventListenerWithSceneGraphPriority(EventListener* list
     addEventListener(listener);
 }
 
-#if COCOS2D_DEBUG > 0
+#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS && COCOS2D_DEBUG > 0
 
 void EventDispatcher::debugCheckNodeHasNoEventListenersOnDestruction(Node* node)
 {
@@ -513,7 +513,7 @@ void EventDispatcher::debugCheckNodeHasNoEventListenersOnDestruction(Node* node)
     }
 }
 
-#endif  // #if COCOS2D_DEBUG > 0
+#endif  // #if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS && COCOS2D_DEBUG > 0
 
 
 void EventDispatcher::addEventListenerWithFixedPriority(EventListener* listener, int fixedPriority)
diff --git a/cocos/2d/CCEventDispatcher.h b/cocos/2d/CCEventDispatcher.h
index 0628e58bea..002f42a224 100644
--- a/cocos/2d/CCEventDispatcher.h
+++ b/cocos/2d/CCEventDispatcher.h
@@ -140,7 +140,7 @@ public:
     /** Destructor of EventDispatcher */
     ~EventDispatcher();
 
-#if COCOS2D_DEBUG > 0
+#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS && COCOS2D_DEBUG > 0
     
     /**
      * To help track down event listener issues in debug builds.
diff --git a/cocos/2d/CCNode.cpp b/cocos/2d/CCNode.cpp
index 4b06df0edf..b739b58074 100644
--- a/cocos/2d/CCNode.cpp
+++ b/cocos/2d/CCNode.cpp
@@ -178,9 +178,9 @@ Node::~Node()
 
 #endif
 
-	#if COCOS2D_DEBUG > 0
-		_eventDispatcher->debugCheckNodeHasNoEventListenersOnDestruction(this);
-	#endif
+#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS && COCOS2D_DEBUG > 0
+    _eventDispatcher->debugCheckNodeHasNoEventListenersOnDestruction(this);
+#endif
 
     CC_SAFE_RELEASE(_eventDispatcher);
 }
diff --git a/cocos/2d/ccConfig.h b/cocos/2d/ccConfig.h
index a58092458f..b9a7ef70a3 100644
--- a/cocos/2d/ccConfig.h
+++ b/cocos/2d/ccConfig.h
@@ -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).

From 824efceb3fe66306f37255a3d3a423d7dbff705e Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 15:14:37 -0700
Subject: [PATCH 05/12] Revert "Review feedback: fix memory leak discovered."

This reverts commit 07646f1c827d289869898fb7f4e5ca7c60d1e711.
---
 cocos/2d/CCEventDispatcher.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index 2d146cce27..ecc7cb3b22 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -566,6 +566,7 @@ void EventDispatcher::removeEventListener(EventListener* listener)
                 if (_inDispatch == 0)
                 {
                     listeners->erase(iter);
+                    CC_SAFE_RELEASE(l);
                 }
                 else
                 {
@@ -576,7 +577,6 @@ void EventDispatcher::removeEventListener(EventListener* listener)
                 }
 
                 isFound = true;
-                CC_SAFE_RELEASE(l);
                 break;
             }
         }

From a778d07ecc761e6a0a0b3ed3db1341aa0be219d6 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 15:22:11 -0700
Subject: [PATCH 06/12] Undo most of the rest of the event dispatcher related
 changes in this fix. Will update with a much more elegant solution that does
 not disturb the existing code so much and which ensures no dangling pointers.

---
 cocos/2d/CCEventDispatcher.cpp | 38 +++-------------------------------
 1 file changed, 3 insertions(+), 35 deletions(-)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index ecc7cb3b22..20ff435504 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -568,14 +568,7 @@ void EventDispatcher::removeEventListener(EventListener* listener)
                     listeners->erase(iter);
                     CC_SAFE_RELEASE(l);
                 }
-                else
-                {
-                	// Fix to prevent events propagating to nodes or listeners that are destroyed:
-                    // Since we can't remove the listener while we're iterating the listeners list,
-                    // null out the pointer. We'll erase the actual pointer storage later.
-                    *iter = nullptr;
-                }
-
+                
                 isFound = true;
                 break;
             }
@@ -706,11 +699,6 @@ void EventDispatcher::dispatchEventToListeners(EventListenerVector* listeners, c
             // priority == 0, scene graph priority
             for (auto& l : *sceneGraphPriorityListeners)
             {
-                if (!l)
-                {
-                    continue;   // Event listener slot which was temporarily nulled because we removed the listener while dispatching events.
-                }
-
                 if (!l->isPaused() && l->isRegistered() && onEvent(l))
                 {
                     shouldStopPropagation = true;
@@ -998,13 +986,7 @@ void EventDispatcher::updateListeners(Event* event)
             for (auto iter = sceneGraphPriorityListeners->begin(); iter != sceneGraphPriorityListeners->end();)
             {
                 auto l = *iter;
-
-                if (!l)
-                {
-                    // Event listener slot which was temporarily nulled because we removed the listener while dispatching events.
-                    iter = sceneGraphPriorityListeners->erase(iter);
-                }
-                else if (!l->isRegistered())
+                if (!l->isRegistered())
                 {
                     iter = sceneGraphPriorityListeners->erase(iter);
                     l->release();
@@ -1021,13 +1003,7 @@ void EventDispatcher::updateListeners(Event* event)
             for (auto iter = fixedPriorityListeners->begin(); iter != fixedPriorityListeners->end();)
             {
                 auto l = *iter;
-
-                if (!l)
-                {
-                    // Event listener slot which was temporarily nulled because we removed the listener while dispatching events.
-                    iter = fixedPriorityListeners->erase(iter);
-                }
-                else if (!l->isRegistered())
+                if (!l->isRegistered())
                 {
                     iter = fixedPriorityListeners->erase(iter);
                     l->release();
@@ -1233,14 +1209,6 @@ void EventDispatcher::removeEventListenersForListenerID(const EventListener::Lis
             for (auto iter = listenerVector->begin(); iter != listenerVector->end();)
             {
                 auto l = *iter;
-
-	            if (!l)
-	            {
-	                // Listener slot which was erased by nulling out the slot.
-	                // This happens when we try to remove a listener while dispatching events.
-	                continue;
-	            }
-				
                 l->setRegistered(false);
                 if (l->getSceneGraphPriority() != nullptr)
                 {

From c6cd927ea5600391de4a5a8f023f0d383b4e2f33 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 15:37:16 -0700
Subject: [PATCH 07/12] When an event listener is removed from the scene and
 has a node associated with it, NULL out the node pointer so as not to leave
 any dangling pointers.

---
 cocos/2d/CCEventDispatcher.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index 20ff435504..9aa7126805 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -561,6 +561,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)

From 83495e6e9bcbb9de437345f6e7484ace1b4c4a2d Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 15:39:52 -0700
Subject: [PATCH 08/12] Macro wrap some of the other debug checks related to
 event listeners.

---
 cocos/2d/CCEventDispatcher.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index 9aa7126805..c9cf4301e6 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -130,6 +130,7 @@ 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!");
@@ -137,6 +138,7 @@ void EventDispatcher::EventListenerVector::push_back(EventListener* listener)
     CCASSERT(_fixedListeners == nullptr ||
              std::count(_fixedListeners->begin(), _fixedListeners->end(), listener) == 0,
              "Listener should not be added twice!");
+#endif
 
     if (listener->getFixedPriority() == 0)
     {
@@ -597,6 +599,7 @@ void EventDispatcher::removeEventListener(EventListener* listener)
             }
         }
         
+#if CC_NODE_DEBUG_VERIFY_EVENT_LISTENERS
         CCASSERT(_inDispatch != 0 ||
                  !sceneGraphPriorityListeners ||
                  std::count(sceneGraphPriorityListeners->begin(), sceneGraphPriorityListeners->end(), listener) == 0,
@@ -605,7 +608,8 @@ void EventDispatcher::removeEventListener(EventListener* listener)
         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.");			 
+                 "Listener should be in no lists after this is done if we're not currently in dispatch mode.");
+#endif
 
         if (iter->second->empty())
         {

From 2b7116060da33195ece11b680f1ad122a9766b61 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 16:00:21 -0700
Subject: [PATCH 09/12] Add a test for events being sent to destroyed nodes.

---
 .../NewEventDispatcherTest.cpp                | 136 +++++++++++++++++-
 .../NewEventDispatcherTest.h                  |  11 ++
 2 files changed, 146 insertions(+), 1 deletion(-)

diff --git a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
index 9852b62323..5d837ac09f 100644
--- a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
+++ b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
@@ -26,7 +26,8 @@ std::function<Layer*()> createFunctions[] =
     CL(StopPropagationTest),
     CL(PauseResumeTargetTest),
     CL(Issue4129),
-    CL(Issue4160)
+    CL(Issue4160),
+    CL(EventsToDestroyedNodeTest)
 };
 
 unsigned int TEST_CASE_COUNT = sizeof(createFunctions) / sizeof(createFunctions[0]);
@@ -1274,3 +1275,136 @@ std::string Issue4160::subtitle() const
 {
     return "Touch the red block twice \n should not crash and the red one couldn't be touched";
 }
+
+// EventsToDestroyedNodeTest
+class EventsToDestroyedNodeTestSprite : public Sprite
+{
+public:
+    
+    typedef std::function<void (EventsToDestroyedNodeTestSprite * sprite)> TappedCallback;
+    
+    static EventsToDestroyedNodeTestSprite * create(const TappedCallback & tappedCallback)
+    {
+        auto ret = new EventsToDestroyedNodeTestSprite(tappedCallback);
+        
+        if (ret && ret->init())
+        {
+            ret->autorelease();
+            return ret;
+        }
+
+        CC_SAFE_DELETE(ret);
+        return nullptr;
+    }
+
+protected:
+    
+    EventsToDestroyedNodeTestSprite(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;
+};
+
+EventsToDestroyedNodeTest::EventsToDestroyedNodeTest()
+{
+    Point origin = Director::getInstance()->getVisibleOrigin();
+    Size size = Director::getInstance()->getVisibleSize();
+    
+    auto callback2 = [](EventsToDestroyedNodeTestSprite * 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](EventsToDestroyedNodeTestSprite * sprite1)
+    {
+        EventsToDestroyedNodeTestSprite * sprite2 = dynamic_cast<EventsToDestroyedNodeTestSprite*>(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 = EventsToDestroyedNodeTestSprite::create(callback2);
+        sprite2->setTexture("Images/MagentaSquare.png");
+        sprite2->setPosition(origin+Point(size.width/2, size.height/2));
+        sprite1->addChild(sprite2, -20);
+    };
+    
+    auto sprite1 = EventsToDestroyedNodeTestSprite::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 = EventsToDestroyedNodeTestSprite::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);
+}
+
+EventsToDestroyedNodeTest::~EventsToDestroyedNodeTest()
+{
+    EventDispatcherTestDemo::onEnter();
+    
+    Point origin = Director::getInstance()->getVisibleOrigin();
+    Size size = Director::getInstance()->getVisibleSize();
+    
+    auto sprite1 = TouchableSprite::create(20);
+    sprite1->setTexture("Images/CyanSquare.png");
+    sprite1->setPosition(origin+Point(size.width/2, size.height/2));
+    addChild(sprite1, 10);
+    
+    auto sprite2 = TouchableSprite::create(10);
+    sprite2->setTexture("Images/MagentaSquare.png");
+    sprite1->setPosition(origin+Point(size.width/2, size.height/2));
+    addChild(sprite2, 20);
+}
+
+std::string EventsToDestroyedNodeTest::title() const
+{
+    return "EventsToDestroyedNodeTest";
+}
+
+std::string EventsToDestroyedNodeTest::subtitle() const
+{
+    return  "1: Goto the next test and return to this test.\n"
+            "2: Tap the square - should not crash!";
+}
diff --git a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h
index e35035b4c6..4e78c6e53a 100644
--- a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h
+++ b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h
@@ -206,4 +206,15 @@ public:
 private:
 };
 
+class EventsToDestroyedNodeTest : public EventDispatcherTestDemo
+{
+public:
+    CREATE_FUNC(EventsToDestroyedNodeTest);
+    EventsToDestroyedNodeTest();
+    virtual ~EventsToDestroyedNodeTest();
+    
+    virtual std::string title() const override;
+    virtual std::string subtitle() const override;
+};
+
 #endif /* defined(__samples__NewEventDispatcherTest__) */

From ba450f786c6c77f31dd610ed1a70e564f1d1e885 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 16:16:27 -0700
Subject: [PATCH 10/12] Fix the new test added to the event dispatcher tests

---
 .../NewEventDispatcherTest.cpp                      | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
index 5d837ac09f..3568d0440f 100644
--- a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
+++ b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
@@ -1382,20 +1382,7 @@ EventsToDestroyedNodeTest::EventsToDestroyedNodeTest()
 
 EventsToDestroyedNodeTest::~EventsToDestroyedNodeTest()
 {
-    EventDispatcherTestDemo::onEnter();
     
-    Point origin = Director::getInstance()->getVisibleOrigin();
-    Size size = Director::getInstance()->getVisibleSize();
-    
-    auto sprite1 = TouchableSprite::create(20);
-    sprite1->setTexture("Images/CyanSquare.png");
-    sprite1->setPosition(origin+Point(size.width/2, size.height/2));
-    addChild(sprite1, 10);
-    
-    auto sprite2 = TouchableSprite::create(10);
-    sprite2->setTexture("Images/MagentaSquare.png");
-    sprite1->setPosition(origin+Point(size.width/2, size.height/2));
-    addChild(sprite2, 20);
 }
 
 std::string EventsToDestroyedNodeTest::title() const

From 8ca3cf8a93b10fe1d444d8d0c6879dc1d526ba29 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 16:38:06 -0700
Subject: [PATCH 11/12] Update the test for dangling node pointers in the
 events system. Should be run when the debug event listener verification for
 nodes is switched on.

---
 .../NewEventDispatcherTest.cpp                | 48 +++++++++++--------
 .../NewEventDispatcherTest.h                  |  8 ++--
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
index 3568d0440f..df2722a935 100644
--- a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
+++ b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.cpp
@@ -27,7 +27,7 @@ std::function<Layer*()> createFunctions[] =
     CL(PauseResumeTargetTest),
     CL(Issue4129),
     CL(Issue4160),
-    CL(EventsToDestroyedNodeTest)
+    CL(DanglingNodePointersTest)
 };
 
 unsigned int TEST_CASE_COUNT = sizeof(createFunctions) / sizeof(createFunctions[0]);
@@ -1276,16 +1276,16 @@ std::string Issue4160::subtitle() const
     return "Touch the red block twice \n should not crash and the red one couldn't be touched";
 }
 
-// EventsToDestroyedNodeTest
-class EventsToDestroyedNodeTestSprite : public Sprite
+// DanglingNodePointersTest
+class DanglingNodePointersTestSprite : public Sprite
 {
 public:
     
-    typedef std::function<void (EventsToDestroyedNodeTestSprite * sprite)> TappedCallback;
+    typedef std::function<void (DanglingNodePointersTestSprite * sprite)> TappedCallback;
     
-    static EventsToDestroyedNodeTestSprite * create(const TappedCallback & tappedCallback)
+    static DanglingNodePointersTestSprite * create(const TappedCallback & tappedCallback)
     {
-        auto ret = new EventsToDestroyedNodeTestSprite(tappedCallback);
+        auto ret = new DanglingNodePointersTestSprite(tappedCallback);
         
         if (ret && ret->init())
         {
@@ -1299,7 +1299,7 @@ public:
 
 protected:
     
-    EventsToDestroyedNodeTestSprite(const TappedCallback & tappedCallback)
+    DanglingNodePointersTestSprite(const TappedCallback & tappedCallback)
     :
         _eventListener(nullptr),
         _tappedCallback(tappedCallback)
@@ -1344,54 +1344,62 @@ private:
     TappedCallback                  _tappedCallback;
 };
 
-EventsToDestroyedNodeTest::EventsToDestroyedNodeTest()
+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 = [](EventsToDestroyedNodeTestSprite * sprite2)
+    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](EventsToDestroyedNodeTestSprite * sprite1)
+    auto callback1 = [callback2, origin, size](DanglingNodePointersTestSprite * sprite1)
     {
-        EventsToDestroyedNodeTestSprite * sprite2 = dynamic_cast<EventsToDestroyedNodeTestSprite*>(sprite1->getChildren().at(0));
+        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 = EventsToDestroyedNodeTestSprite::create(callback2);
+        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 = EventsToDestroyedNodeTestSprite::create(callback1);    // Sprite 1 will receive touch before sprite 2
+    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 = EventsToDestroyedNodeTestSprite::create(callback2);   // Sprite 2 will be removed when sprite 1 is touched, should never receive an event.
+    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
 }
 
-EventsToDestroyedNodeTest::~EventsToDestroyedNodeTest()
+DanglingNodePointersTest::~DanglingNodePointersTest()
 {
     
 }
 
-std::string EventsToDestroyedNodeTest::title() const
+std::string DanglingNodePointersTest::title() const
 {
-    return "EventsToDestroyedNodeTest";
+    return "DanglingNodePointersTest";
 }
 
-std::string EventsToDestroyedNodeTest::subtitle() const
+std::string DanglingNodePointersTest::subtitle() const
 {
-    return  "1: Goto the next test and return to this test.\n"
-            "2: Tap the square - should not crash!";
+#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
 }
diff --git a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h
index 4e78c6e53a..74c692d49c 100644
--- a/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h
+++ b/tests/cpp-tests/Classes/NewEventDispatcherTest/NewEventDispatcherTest.h
@@ -206,12 +206,12 @@ public:
 private:
 };
 
-class EventsToDestroyedNodeTest : public EventDispatcherTestDemo
+class DanglingNodePointersTest : public EventDispatcherTestDemo
 {
 public:
-    CREATE_FUNC(EventsToDestroyedNodeTest);
-    EventsToDestroyedNodeTest();
-    virtual ~EventsToDestroyedNodeTest();
+    CREATE_FUNC(DanglingNodePointersTest);
+    DanglingNodePointersTest();
+    virtual ~DanglingNodePointersTest();
     
     virtual std::string title() const override;
     virtual std::string subtitle() const override;

From 0f729126f070f96d1e9a8b0bc1681904033084d6 Mon Sep 17 00:00:00 2001
From: Darragh Coy <dcoy@playfirst.com>
Date: Thu, 27 Mar 2014 20:11:37 -0700
Subject: [PATCH 12/12] Find another instance where we should null out the node
 pointer for the current listener after it has been disassociated with the
 node.

---
 cocos/2d/CCEventDispatcher.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cocos/2d/CCEventDispatcher.cpp b/cocos/2d/CCEventDispatcher.cpp
index c9cf4301e6..38ef104175 100644
--- a/cocos/2d/CCEventDispatcher.cpp
+++ b/cocos/2d/CCEventDispatcher.cpp
@@ -1218,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)