Fix bugs in de::AppendList

 * de::AppendList was not calling destructor for elements in the last
block. This would result in resource leak if elements have non-trivial
destructor.

 * de::AppendList::clear() failed to set m_first->next to null resulting
use of freed blocks.

Bug: 27909093
Change-Id: Id012bd6c76eb31058b302d0540891e5280a2d39f

diff --git a/framework/delibs/decpp/deAppendList.cpp b/framework/delibs/decpp/deAppendList.cpp
index ed7ef74..7433173 100644 (file)
--- a/framework/delibs/decpp/deAppendList.cpp
+++ b/framework/delibs/decpp/deAppendList.cpp
@@ -132,6 +132,62 @@ void runAppendListTest (deUint32 numThreads, deUint32 numElements, deUint32 numE
        }
 }
 
+class ObjCountElem
+{
+public:
+       ObjCountElem (int* liveCount)
+               : m_liveCount(liveCount)
+       {
+               *m_liveCount += 1;
+       }
+
+       ~ObjCountElem (void)
+       {
+               *m_liveCount -= 1;
+       }
+
+       ObjCountElem (const ObjCountElem& other)
+               : m_liveCount(other.m_liveCount)
+       {
+               *m_liveCount += 1;
+       }
+
+       ObjCountElem& operator= (const ObjCountElem& other)
+       {
+               m_liveCount = other.m_liveCount;
+               *m_liveCount += 1;
+               return *this;
+       }
+
+private:
+       int* m_liveCount;
+};
+
+void runClearTest (deUint32 numElements1, deUint32 numElements2, deUint32 numElementsHint)
+{
+       int             liveCount       = 0;
+
+       {
+               de::AppendList<ObjCountElem>    testList        (numElementsHint);
+
+               for (deUint32 ndx = 0; ndx < numElements1; ++ndx)
+                       testList.append(ObjCountElem(&liveCount));
+
+               DE_TEST_ASSERT(liveCount == (int)numElements1);
+
+               testList.clear();
+
+               DE_TEST_ASSERT(liveCount == 0);
+
+               for (deUint32 ndx = 0; ndx < numElements2; ++ndx)
+                       testList.append(ObjCountElem(&liveCount));
+
+               DE_TEST_ASSERT(liveCount == (int)numElements2);
+       }
+
+       DE_TEST_ASSERT(liveCount == 0);
+}
+
 } // anonymous
 
 void AppendList_selfTest (void)
@@ -150,6 +206,15 @@ void AppendList_selfTest (void)
                runAppendListTest(4, 10000, 500);
                runAppendListTest(4, 100, 10);
        }
+
+       // Dtor + clear()
+       runClearTest(1, 1, 1);
+       runClearTest(1, 2, 10);
+       runClearTest(50, 25, 10);
+       runClearTest(9, 50, 10);
+       runClearTest(10, 50, 10);
+       runClearTest(50, 9, 10);
+       runClearTest(50, 10, 10);
 }
 
 } // de

diff --git a/framework/delibs/decpp/deAppendList.hpp b/framework/delibs/decpp/deAppendList.hpp
index 6ad3bb4..21c9f67 100644 (file)
--- a/framework/delibs/decpp/deAppendList.hpp
+++ b/framework/delibs/decpp/deAppendList.hpp
@@ -182,7 +182,7 @@ AppendList<ElementType>::~AppendList (void)
                curBlock = delBlock->next;
 
                // Call destructor for allocated elements
-               for (; elementNdx < min(m_numElements, delBlock->blockNdx*m_blockSize); ++elementNdx)
+               for (; elementNdx < min(m_numElements, (delBlock->blockNdx+1)*m_blockSize); ++elementNdx)
                        delBlock->elements[elementNdx%m_blockSize].~ElementType();
 
                delete delBlock;
@@ -206,7 +206,7 @@ void AppendList<ElementType>::clear (void)
                curBlock = delBlock->next;
 
                // Call destructor for allocated elements
-               for (; elementNdx < min(m_numElements, delBlock->blockNdx*m_blockSize); ++elementNdx)
+               for (; elementNdx < min(m_numElements, (delBlock->blockNdx+1)*m_blockSize); ++elementNdx)
                        delBlock->elements[elementNdx%m_blockSize].~ElementType();
 
                if (delBlock != m_first)
@@ -216,6 +216,7 @@ void AppendList<ElementType>::clear (void)
        DE_ASSERT(elementNdx == m_numElements);
 
        m_numElements   = 0;
+       m_first->next   = DE_NULL;
        m_last                  = m_first;
 }