Memory leak in all MoPubAdRenderer's WeakHashMap


#1

Hi, it looks like there’s a classical misuse of WeakHashMap in the way you’re storing the StaticNativeViewHolder instances:

StaticNativeViewHolder staticNativeViewHolder = (StaticNativeViewHolder)this.mViewHolderMap.get(view);
        if(staticNativeViewHolder == null) {
            staticNativeViewHolder = StaticNativeViewHolder.fromViewBinder(view, this.mViewBinder);
            this.mViewHolderMap.put(view, staticNativeViewHolder);
        }

Here we have instance of View as a Key for the WeakHashMap so it is presumably kept via WeakReference, but also this View instance is injected in StaticNativeViewHolder which is a Value. So the situation is that the Value holds strong reference to a Key which prevents Views from being collected.

check out this note in the Android docs:

Implementation note: The value objects in a WeakHashMap are held by ordinary strong references. Thus care should be taken to ensure that value objects do not strongly refer to their own keys, either directly or indirectly, since that will prevent the keys from being discarded. Note that a value object may refer indirectly to its key via the WeakHashMap itself; that is, a value object may strongly refer to some other key object whose associated value object, in turn, strongly refers to the key of the first value object. If the values in the map do not rely on the map holding strong references to them, one way to deal with this is to wrap values themselves within WeakReferences before inserting, as in: m.put(key, new WeakReference(value)), and then unwrapping upon each get.

Not sure how others didnt notice this leak yet, maybe i’m doing something wrong, but it seems pretty straightforward :slight_smile:


#2

Hello,

Thanks for surfacing this leak. We are aware of it, and should be releasing a fix soon.