Memory Leak in practical example

Question

I am constantly struggling with identifying memory leaks. I guess I have several memory leaks in my project circular progress view.

One of my guesses is I have a memory leak in the internal class FadeRunnable. But to be honest I don't know exactly how to find out if this is exactly the source of the problem. Well, when I do the usual scenario and switch the orientation I see an increase of the memory usage as shown below. And if I comment out the usage of the FadeRunnable class the steps are smaller (but still there, so I guess that's not the only leak)

Once I analyze the heap dump, I see something. But actually I don't know what the values mean. The things I do is

1. Change orientation many times
2. Open heap dump and sort by 'Retained Size'
3. Now when I click on "CircularProgressView' I see 8 rows in the right area. I guess this means there are 8 instances of the 'CircularProgressView' leaked and living somewhere as orphan in the memory.

Is this correct? If so, how can I find out in the dump information (I guess somewhere in the lower pane) where this object is saved/held.

I would love to have a step-by-step explanation how to find out if and which object is leaking some memory.

All of the code of the suspected view can be found in this class.

https://github.com/momentummodules/CircularProgressView/blob/master/circularprogressview/src/main/java/momentum/circularprogressview/CircularProgressView.java

But also feel free to check out the full project for deeper insight and if you want to play around with it.

Thanks in advance!

UPDATE

The code link from above shows the fixed code of the mem-leaking inner class. The following snippet shows the original mem-leaking code that should never be used like that

/**
 * Mem-leaking code, for fixed code see repository link
 * https://github.com/momentummodules/CircularProgressView/blob/master/circularprogressview/src/main/java/momentum/circularprogressview/CircularProgressView.java
 */
public class CircularProgressView extends View
{
    ...
    private Thread fadeThread = null;
    ...

    ...
    class FadeRunnable implements Runnable
    {
        @Override
        public void run()
        {
            ...
        }
    }
    ...

    ...   
    private void startFade(boolean fadeIn)
    {
        // check existing
        if(this.fadeThread != null)
        {
            // check if fade is already running
            switch(this.fadeThread.getState())
            {
                case TERMINATED:
                case NEW:
                    this.fadeThread = null;
                    break;
                case RUNNABLE:
                case BLOCKED:
                case TIMED_WAITING:
                case WAITING:
                    return;
            }
        }
        // create new
        this.fadeThread = new Thread(new FadeRunnable(fadeIn, this.fadeTime));
        this.fadeThread.start();
    }
}

Answer 1

Yes, you do have a memory leak in FadeRunnable class.

Every instance of inner class contains implicit reference to its outer class, accessible through OuterClass.this operator. In your project, when you execute the FadeRunnable and then trigger reconfiguration by orientation change, the whole activity and your CircularProgressView contained within get recreated, but the FadeRunnable from previous is still alive (allocated) and, because of it holding implicit reference to its outer CircularProgressView class, the view continues to live also, that's why after several reconfigurations you have 8 instances of CircularProgressView allocated in memory, and that gets worse - every View keeps a reference to it's context, and this cannot be freed also, resulting in bad memory leaks.

Runnables, Handlers and similar objects that can out-live their enclosing activities, fragments, views etc. should be declared as standard classes or STATIC inner classes (a static inner class doesn't hold implicit reference to its outer class), and shouldn't keep references such as Context, View etc., instead you can keep a WeakReference<> so when your Activity is recreated through config change, the View can be destroyed and freed by garbage collector.

This is a very informative article on the subject, I strongly suggest reading it.