Merge pull request #5203 from dumganhar/iss3713-ref-patch

issue #3713: Object::release() of debug version will check whether the object was in autorelease pool.

cocos/base/CCAutoreleasePool.cpp

@@ -28,14 +28,20 @@ THE SOFTWARE.
 NS_CC_BEGIN
 
 AutoreleasePool::AutoreleasePool()
-:_name("")
+: _name("")
+#if defined(COCOS2D_DEBUG) && (COCOS2D_DEBUG > 0)
+, _isClearing(false)
+#endif
 {
     _managedObjectArray.reserve(150);
     PoolManager::getInstance()->push(this);
 }
 
 AutoreleasePool::AutoreleasePool(const std::string &name)
-:_name(name)
+: _name(name)
+#if defined(COCOS2D_DEBUG) && (COCOS2D_DEBUG > 0)
+, _isClearing(false)
+#endif
 {
     _managedObjectArray.reserve(150);
     PoolManager::getInstance()->push(this);
@@ -56,11 +62,27 @@ void AutoreleasePool::addObject(Object* object)
 
 void AutoreleasePool::clear()
 {
+#if defined(COCOS2D_DEBUG) && (COCOS2D_DEBUG > 0)
+    _isClearing = true;
+#endif
     for (const auto &obj : _managedObjectArray)
     {
         obj->release();
     }
     _managedObjectArray.clear();
+#if defined(COCOS2D_DEBUG) && (COCOS2D_DEBUG > 0)
+    _isClearing = false;
+#endif
+}
+
+bool AutoreleasePool::contains(Object* object) const
+{
+    for (const auto& obj : _managedObjectArray)
+    {
+        if (obj == object)
+            return true;
+    }
+    return false;
 }
 
 void AutoreleasePool::dump()
@@ -89,7 +111,7 @@ PoolManager* PoolManager::getInstance()
         s_singleInstance = new PoolManager();
         // Add the first auto release pool
         s_singleInstance->_curReleasePool = new AutoreleasePool("cocos2d autorelease pool");
-        s_singleInstance->_releasePoolStack.push(s_singleInstance->_curReleasePool);
+        s_singleInstance->_releasePoolStack.push_back(s_singleInstance->_curReleasePool);
     }
     return s_singleInstance;
 }
@@ -110,8 +132,8 @@ PoolManager::~PoolManager()
     
     while (!_releasePoolStack.empty())
     {
-        AutoreleasePool* pool = _releasePoolStack.top();
-        _releasePoolStack.pop();
+        AutoreleasePool* pool = _releasePoolStack.back();
+        _releasePoolStack.pop_back();
         
         delete pool;
     }
@@ -123,9 +145,19 @@ AutoreleasePool* PoolManager::getCurrentPool() const
     return _curReleasePool;
 }
 
+bool PoolManager::isObjectInPools(Object* obj) const
+{
+    for (const auto& pool : _releasePoolStack)
+    {
+        if (pool->contains(obj))
+            return true;
+    }
+    return false;
+}
+
 void PoolManager::push(AutoreleasePool *pool)
 {
-    _releasePoolStack.push(pool);
+    _releasePoolStack.push_back(pool);
     _curReleasePool = pool;
 }
 
@@ -134,12 +166,12 @@ void PoolManager::pop()
     // Can not pop the pool that created by engine
     CC_ASSERT(_releasePoolStack.size() >= 1);
     
-    _releasePoolStack.pop();
+    _releasePoolStack.pop_back();
     
     // Should update _curReleasePool if a temple pool is released
     if (_releasePoolStack.size() > 1)
     {
-        _curReleasePool = _releasePoolStack.top();
+        _curReleasePool = _releasePoolStack.back();
     }
 }
 

cocos/base/CCAutoreleasePool.h

@@ -81,6 +81,18 @@ public:
      */
     void clear();
     
+#if defined(COCOS2D_DEBUG) && (COCOS2D_DEBUG > 0)
+    /**
+     * Whether the pool is doing `clear` operation.
+     */
+    bool isClearing() const { return _isClearing; };
+#endif
+    
+    /**
+     * Checks whether the pool contains the specified object.
+     */
+    bool contains(Object* object) const;
+
     /**
      * Dump the objects that are put into autorelease pool. It is used for debugging.
      *
@@ -102,6 +114,13 @@ private:
      */
     std::vector<Object*> _managedObjectArray;
     std::string _name;
+    
+#if defined(COCOS2D_DEBUG) && (COCOS2D_DEBUG > 0)
+    /**
+     *  The flag for checking whether the pool is doing `clear` operation.
+     */
+    bool _isClearing;
+#endif
 };
 
 class CC_DLL PoolManager
@@ -127,6 +146,8 @@ public:
      */
     AutoreleasePool *getCurrentPool() const;
 
+    bool isObjectInPools(Object* obj) const;
+
     /**
      * @js NA
      * @lua NA
@@ -142,7 +163,7 @@ private:
     
     static PoolManager* s_singleInstance;
     
-    std::stack<AutoreleasePool*> _releasePoolStack;
+    std::deque<AutoreleasePool*> _releasePoolStack;
     AutoreleasePool *_curReleasePool;
 };
 

cocos/base/CCObject.cpp

@@ -63,6 +63,52 @@ Object* Object::autorelease()
     return this;
 }
 
+
+#if defined(COCOS2D_DEBUG) && (COCOS2D_DEBUG > 0)
+void Object::release()
+{
+    CCASSERT(_referenceCount > 0, "reference count should greater than 0");
+    --_referenceCount;
+    
+    if (_referenceCount == 0)
+    {
+        auto poolManager = PoolManager::getInstance();
+        if (!poolManager->getCurrentPool()->isClearing() && poolManager->isObjectInPools(this))
+        {
+            // Trigger an assert if the reference count is 0 but the object is still in autorelease pool.
+            // This happens when 'autorelease/release' were not used in pairs with 'new/retain'.
+            //
+            // Wrong usage (1):
+            //
+            // auto obj = Node::create();   // Ref = 1, but it's an autorelease object which means it was in the autorelease pool.
+            // obj->autorelease();   // Wrong: If you wish to invoke autorelease several times, you should retain `obj` first.
+            //
+            // Wrong usage (2):
+            //
+            // auto obj = Node::create();
+            // obj->release();   // Wrong: obj is an autorelease object, it will be released when clearing current pool.
+            //
+            // Correct usage (1):
+            //
+            // auto obj = Node::create();
+            //                     |-   new Node();     // `new` is the pair of the `autorelease` of next line
+            //                     |-   autorelease();  // The pair of `new Node`.
+            //
+            // obj->retain();
+            // obj->autorelease();  // This `autorelease` is the pair of `retain` of previous line.
+            //
+            // Corrent usage (2):
+            //
+            // auto obj = Node::create();
+            // obj->retain();
+            // obj->release();   // This `release` is the pair of `retain` of previous line.
+            CCASSERT(false, "The reference shouldn't be 0 because it is still in autorelease pool.");
+        }
+        delete this;
+    }
+}
+#endif
+
 bool Object::isSingleReference() const
 {
     return _referenceCount == 1;

cocos/base/CCObject.h

@@ -103,15 +103,16 @@ public:
      * @see retain, autorelease
      * @js NA
      */
+#if defined(COCOS2D_DEBUG) && (COCOS2D_DEBUG > 0)
+    void release();
+#else
     inline void release()
     {
-        CCASSERT(_referenceCount > 0, "reference count should greater than 0");
         --_referenceCount;
-
         if (_referenceCount == 0)
             delete this;
     }
-
+#endif
     /**
      * Retains the ownership.
      *