Fix a leak in RefBase (DO NOT MERGE)

this bug was introduced recently. it caused RefBase's weakref_impl
structure to be leaked for every RefBase object (about 20 bytes).

Change-Id: Ia9b155fbfa643ef72cfb8129e96260a3b806a78c

1 files changed, 80 insertions, 151 deletions

diff --git a/libs/utils/RefBase.cpp b/libs/utils/RefBase.cpp
index dd0052a..1c870cf 100644
--- a/libs/utils/RefBase.cpp
+++ b/libs/utils/RefBase.cpp
@@ -20,9 +20,9 @@
 
 #include <utils/Atomic.h>
 #include <utils/CallStack.h>
+#include <utils/KeyedVector.h>
 #include <utils/Log.h>
 #include <utils/threads.h>
-#include <utils/TextOutput.h>
 
 #include <stdlib.h>
 #include <stdio.h>
@@ -34,7 +34,6 @@
 
 // compile with refcounting debugging enabled
 #define DEBUG_REFS                      0
-#define DEBUG_REFS_FATAL_SANITY_CHECKS  0
 #define DEBUG_REFS_ENABLED_BY_DEFAULT   1
 #define DEBUG_REFS_CALLSTACK_ENABLED    1
 
@@ -70,10 +69,8 @@ public:
 
     void addStrongRef(const void* /*id*/) { }
     void removeStrongRef(const void* /*id*/) { }
-    void renameStrongRefId(const void* /*old_id*/, const void* /*new_id*/) { }
     void addWeakRef(const void* /*id*/) { }
     void removeWeakRef(const void* /*id*/) { }
-    void renameWeakRefId(const void* /*old_id*/, const void* /*new_id*/) { }
     void printRefs() const { }
     void trackMe(bool, bool) { }
 
@@ -89,91 +86,39 @@ public:
         , mTrackEnabled(!!DEBUG_REFS_ENABLED_BY_DEFAULT)
         , mRetain(false)
     {
+        //LOGI("NEW weakref_impl %p for RefBase %p", this, base);
     }
     
     ~weakref_impl()
     {
-        bool dumpStack = false;
-        if (!mRetain && mStrongRefs != NULL) {
-            dumpStack = true;
-#if DEBUG_REFS_FATAL_SANITY_CHECKS
-            LOG_ALWAYS_FATAL("Strong references remain!");
-#else
-            LOGE("Strong references remain:");
-#endif
-            ref_entry* refs = mStrongRefs;
-            while (refs) {
-                char inc = refs->ref >= 0 ? '+' : '-';
-                LOGD("\t%c ID %p (ref %d):", inc, refs->id, refs->ref);
-#if DEBUG_REFS_CALLSTACK_ENABLED
-                refs->stack.dump();
-#endif;
-                refs = refs->next;
-            }
-        }
-
-        if (!mRetain && mWeakRefs != NULL) {
-            dumpStack = true;
-#if DEBUG_REFS_FATAL_SANITY_CHECKS
-            LOG_ALWAYS_FATAL("Weak references remain:");
-#else
-            LOGE("Weak references remain!");
-#endif
-            ref_entry* refs = mWeakRefs;
-            while (refs) {
-                char inc = refs->ref >= 0 ? '+' : '-';
-                LOGD("\t%c ID %p (ref %d):", inc, refs->id, refs->ref);
-#if DEBUG_REFS_CALLSTACK_ENABLED
-                refs->stack.dump();
-#endif;
-                refs = refs->next;
-            }
-        }
-        if (dumpStack) {
-            LOGE("above errors at:");
-            CallStack stack;
-            stack.update();
-            stack.dump();
-        }
+        LOG_ALWAYS_FATAL_IF(!mRetain && mStrongRefs != NULL, "Strong references remain!");
+        LOG_ALWAYS_FATAL_IF(!mRetain && mWeakRefs != NULL, "Weak references remain!");
     }
 
-    void addStrongRef(const void* id) {
-        //LOGD_IF(mTrackEnabled,
-        //        "addStrongRef: RefBase=%p, id=%p", mBase, id);
+    void addStrongRef(const void* id)
+    {
         addRef(&mStrongRefs, id, mStrong);
     }
 
-    void removeStrongRef(const void* id) {
-        //LOGD_IF(mTrackEnabled,
-        //        "removeStrongRef: RefBase=%p, id=%p", mBase, id);
-        if (!mRetain) {
+    void removeStrongRef(const void* id)
+    {
+        if (!mRetain)
             removeRef(&mStrongRefs, id);
-        } else {
+        else
             addRef(&mStrongRefs, id, -mStrong);
-        }
     }
 
-    void renameStrongRefId(const void* old_id, const void* new_id) {
-        //LOGD_IF(mTrackEnabled,
-        //        "renameStrongRefId: RefBase=%p, oid=%p, nid=%p",
-        //        mBase, old_id, new_id);
-        renameRefsId(mStrongRefs, old_id, new_id);
-    }
-
-    void addWeakRef(const void* id) {
+    void addWeakRef(const void* id)
+    {
         addRef(&mWeakRefs, id, mWeak);
     }
 
-    void removeWeakRef(const void* id) {
-        if (!mRetain) {
+    void removeWeakRef(const void* id)
+    {
+        if (!mRetain)
             removeRef(&mWeakRefs, id);
-        } else {
+        else
             addRef(&mWeakRefs, id, -mWeak);
-        }
-    }
-
-    void renameWeakRefId(const void* old_id, const void* new_id) {
-        renameRefsId(mWeakRefs, old_id, new_id);
     }
 
     void trackMe(bool track, bool retain)
@@ -187,7 +132,8 @@ public:
         String8 text;
 
         {
-            Mutex::Autolock _l(const_cast<weakref_impl*>(this)->mMutex);
+            AutoMutex _l(const_cast<weakref_impl*>(this)->mMutex);
+    
             char buf[128];
             sprintf(buf, "Strong references on RefBase %p (weakref_type %p):\n", mBase, this);
             text.append(buf);
@@ -226,7 +172,6 @@ private:
     {
         if (mTrackEnabled) {
             AutoMutex _l(mMutex);
-
             ref_entry* ref = new ref_entry;
             // Reference count at the time of the snapshot, but before the
             // update.  Positive value means we increment, negative--we
@@ -236,6 +181,7 @@ private:
 #if DEBUG_REFS_CALLSTACK_ENABLED
             ref->stack.update(2);
 #endif
+            
             ref->next = *refs;
             *refs = ref;
         }
@@ -246,52 +192,20 @@ private:
         if (mTrackEnabled) {
             AutoMutex _l(mMutex);
             
-            ref_entry* const head = *refs;
-            ref_entry* ref = head;
+            ref_entry* ref = *refs;
             while (ref != NULL) {
                 if (ref->id == id) {
                     *refs = ref->next;
                     delete ref;
                     return;
                 }
+                
                 refs = &ref->next;
                 ref = *refs;
             }
-
-#if DEBUG_REFS_FATAL_SANITY_CHECKS
-            LOG_ALWAYS_FATAL("RefBase: removing id %p on RefBase %p"
-                    "(weakref_type %p) that doesn't exist!",
-                    id, mBase, this);
-#endif
-
-            LOGE("RefBase: removing id %p on RefBase %p"
-                    "(weakref_type %p) that doesn't exist!",
-                    id, mBase, this);
-
-            ref = head;
-            while (ref) {
-                char inc = ref->ref >= 0 ? '+' : '-';
-                LOGD("\t%c ID %p (ref %d):", inc, ref->id, ref->ref);
-                ref = ref->next;
-            }
-
-            CallStack stack;
-            stack.update();
-            stack.dump();
-        }
-    }
-
-    void renameRefsId(ref_entry* r, const void* old_id, const void* new_id)
-    {
-        if (mTrackEnabled) {
-            AutoMutex _l(mMutex);
-            ref_entry* ref = r;
-            while (ref != NULL) {
-                if (ref->id == old_id) {
-                    ref->id = new_id;
-                }
-                ref = ref->next;
-            }
+            
+            LOG_ALWAYS_FATAL("RefBase: removing id %p on RefBase %p (weakref_type %p) that doesn't exist!",
+                             id, mBase, this);
         }
     }
 
@@ -321,6 +235,44 @@ private:
     // on removeref that match the address ones.
     bool mRetain;
 
+#if 0
+    void addRef(KeyedVector<const void*, int32_t>* refs, const void* id)
+    {
+        AutoMutex _l(mMutex);
+        ssize_t i = refs->indexOfKey(id);
+        if (i >= 0) {
+            ++(refs->editValueAt(i));
+        } else {
+            i = refs->add(id, 1);
+        }
+    }
+
+    void removeRef(KeyedVector<const void*, int32_t>* refs, const void* id)
+    {
+        AutoMutex _l(mMutex);
+        ssize_t i = refs->indexOfKey(id);
+        LOG_ALWAYS_FATAL_IF(i < 0, "RefBase: removing id %p that doesn't exist!", id);
+        if (i >= 0) {
+            int32_t val = --(refs->editValueAt(i));
+            if (val == 0) {
+                refs->removeItemsAt(i);
+            }
+        }
+    }
+
+    void printRefs(const KeyedVector<const void*, int32_t>& refs)
+    {
+        const size_t N=refs.size();
+        for (size_t i=0; i<N; i++) {
+            printf("\tID %p: %d remain\n", refs.keyAt(i), refs.valueAt(i));
+        }
+    }
+
+    mutable Mutex mMutex;
+    KeyedVector<const void*, int32_t> mStrongRefs;
+    KeyedVector<const void*, int32_t> mWeakRefs;
+#endif
+
 #endif
 };
 
@@ -329,6 +281,7 @@ private:
 void RefBase::incStrong(const void* id) const
 {
     weakref_impl* const refs = mRefs;
+    refs->addWeakRef(id);
     refs->incWeak(id);
     
     refs->addStrongRef(id);
@@ -364,12 +317,14 @@ void RefBase::decStrong(const void* id) const
             destroy();
         }
     }
+    refs->removeWeakRef(id);
     refs->decWeak(id);
 }
 
 void RefBase::forceIncStrong(const void* id) const
 {
     weakref_impl* const refs = mRefs;
+    refs->addWeakRef(id);
     refs->incWeak(id);
     
     refs->addStrongRef(id);
@@ -418,18 +373,20 @@ void RefBase::weakref_type::decWeak(const void* id)
     if (c != 1) return;
     
     if ((impl->mFlags&OBJECT_LIFETIME_WEAK) != OBJECT_LIFETIME_WEAK) {
-        if (impl->mStrong == INITIAL_STRONG_VALUE)
-            if (impl->mBase)
+        if (impl->mStrong == INITIAL_STRONG_VALUE) {
+            if (impl->mBase) {
                 impl->mBase->destroy();
-        else {
+            }
+        } else {
             // LOGV("Freeing refs %p of old RefBase %p\n", this, impl->mBase);
             delete impl;
         }
     } else {
         impl->mBase->onLastWeakRef(id);
         if ((impl->mFlags&OBJECT_LIFETIME_FOREVER) != OBJECT_LIFETIME_FOREVER) {
-            if (impl->mBase)
+            if (impl->mBase) {
                 impl->mBase->destroy();
+            }
         }
     }
 }
@@ -483,6 +440,7 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
         }
     }
     
+    impl->addWeakRef(id);
     impl->addStrongRef(id);
 
 #if PRINT_REFS
@@ -500,7 +458,7 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
 bool RefBase::weakref_type::attemptIncWeak(const void* id)
 {
     weakref_impl* const impl = static_cast<weakref_impl*>(this);
-
+    
     int32_t curCount = impl->mWeak;
     LOG_ASSERT(curCount >= 0, "attemptIncWeak called on %p after underflow",
                this);
@@ -530,7 +488,7 @@ void RefBase::weakref_type::printRefs() const
 
 void RefBase::weakref_type::trackMe(bool enable, bool retain)
 {
-    static_cast<weakref_impl*>(this)->trackMe(enable, retain);
+    static_cast<const weakref_impl*>(this)->trackMe(enable, retain);
 }
 
 RefBase::weakref_type* RefBase::createWeak(const void* id) const
@@ -547,12 +505,15 @@ RefBase::weakref_type* RefBase::getWeakRefs() const
 RefBase::RefBase()
     : mRefs(new weakref_impl(this))
 {
+//    LOGV("Creating refs %p with RefBase %p\n", mRefs, this);
 }
 
 RefBase::~RefBase()
 {
-    if (mRefs->mWeak == 0) {
-        delete mRefs;
+    if ((mRefs->mFlags & OBJECT_LIFETIME_WEAK) == OBJECT_LIFETIME_WEAK) {
+        if (mRefs->mWeak == 0) {
+            delete mRefs;
+        }
     }
 }
 
@@ -577,37 +538,5 @@ bool RefBase::onIncStrongAttempted(uint32_t flags, const void* id)
 void RefBase::onLastWeakRef(const void* /*id*/)
 {
 }
-
-// ---------------------------------------------------------------------------
-
-void RefBase::moveReferences(void* dst, void const* src, size_t n,
-        const ReferenceConverterBase& caster)
-{
-#if DEBUG_REFS
-    const size_t itemSize = caster.getReferenceTypeSize();
-    for (size_t i=0 ; i<n ; i++) {
-        void*       d = reinterpret_cast<void      *>(intptr_t(dst) + i*itemSize);
-        void const* s = reinterpret_cast<void const*>(intptr_t(src) + i*itemSize);
-        RefBase* ref(reinterpret_cast<RefBase*>(caster.getReferenceBase(d)));
-        ref->mRefs->renameStrongRefId(s, d);
-        ref->mRefs->renameWeakRefId(s, d);
-    }
-#endif
-}
-
-// ---------------------------------------------------------------------------
-
-TextOutput& printStrongPointer(TextOutput& to, const void* val)
-{
-    to << "sp<>(" << val << ")";
-    return to;
-}
-
-TextOutput& printWeakPointer(TextOutput& to, const void* val)
-{
-    to << "wp<>(" << val << ")";
-    return to;
-}
-
-
+        
 }; // namespace android