Why do threads leak on Android?

Question

I've been noticing in our Android app that every time we exit to the home screen we increase the heap size (leak) by the amount of the ByteArrayOutputStream. The best I have been able to manage is by adding

this.mByteArrayOutputStream = null;

at the end of run() to prevent the heap size increasing constantly. If anyone could enlighten me I would be very appreciative. I wrote out the following example that illustrates the problem.

MainActivity.java

public class MainActivity extends Activity {
    private Controller mController;

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);        
    }

    @Override
    public boolean onCreateOptionsMenu(Menu menu) {
        getMenuInflater().inflate(R.menu.activity_main, menu);
        return true;
    }

    @Override
    protected void onStart() {
        super.onStart();

        this.mController = new Controller();
        mController.connect();
    }

    @Override
    protected void onStop() {
        super.onStop();

        mController.quit();
    }
}

Controller.java

public class Controller {
    public volatile ReaderThread mThread;

    public Controller() {
        super();
    }

    public void connect() {
        mThread = new ReaderThread("ReaderThread");
        mThread.start();
    }

    public void quit() {
        mThread.quit();
    }

    public static class ReaderThread extends Thread {
        private volatile boolean isProcessing;
        private ByteArrayOutputStream mByteArrayOutputStream;

        public ReaderThread(String threadName) {
            super(threadName);  
        }

        @Override
        public void run() {
            this.isProcessing = true;

            Log.d(getClass().getCanonicalName(), "START");
            this.mByteArrayOutputStream = new ByteArrayOutputStream(2048000);

            int i = 0;
            while (isProcessing) {
                Log.d(getClass().getCanonicalName(), "Iteration: " + i++);
                mByteArrayOutputStream.write(1);
                try {
                    Thread.sleep(1000);
                } catch (InterruptedException e) {
                }
            }

            try {
                mByteArrayOutputStream.reset();
                mByteArrayOutputStream.close();
            } catch (IOException e) {
                e.printStackTrace();
            }

            Log.d(getClass().getCanonicalName(), "STOP");
        }

        public void quit() {
            this.isProcessing = false;
        }
    }
}

Answer 1

Based on the code that you have shown use, the ByteArrayOutputStream instance can only leak if the ReaderThread instance leaks. And that can only occur if either the thread is still running, or there is still a reachable reference to it somewhere.

Focus on figuring out how the ReaderThread instance leaks.

Answer 2

I´ve had a similar problem and I solved it by nulling the thread.

I've tried your code in the emulator (SDK 16) changing quit()method as follow:

public void quit() { 
    mThread.quit(); 
    mThread = null;  // add this line
} 

I've checked in DDMS that the leak as effectively stopped. Removing the line, the leak returns.

--EDITED--

I've tried this also in a real device using SDK 10, and I'm putting the results bellow.

Did you try the thread nulling in your test code or in you full code? It seams that the test code, don't leak after nulling the thread, neither in a real device or in emulator.

Screen shot after initial start (allocated 7.2MB / used: 4.6MB):

Screen shot after a couple of restarts (allocated 7.2MB / used: 4.6MB):

Screen shot after several and very fast restarts, and rotating device servral times (allocated 13.2MB / used: 4.6MB):

Although memory allocated in the last screen is significantly higher then initial memory, the allocated memory remains 4.6MB, so it´s not leaking.

Answer 3

This problem is pretty interesting. I took a look at it and managed to get a working solution with no leaks. I replaced the volatile variables with their atomic counterparts. I also used an AsyncTask instead of a Thread. That seems to have done the trick. No more leaks.

Code

class Controller {
    public ReaderThread mThread;
    private AtomicBoolean isProcessing = new AtomicBoolean(false);
    public Controller() {
        super();
    }

    public void connect() {
        ReaderThread readerThread = new ReaderThread("ReaderThread",isProcessing);
        readerThread.execute(new Integer[0]);
    }

    public void quit() {
        isProcessing.set(false);
    }

    public static class ReaderThread extends AsyncTask<Integer, Integer, Integer> {
        private AtomicBoolean isProcessing;
        private ByteArrayOutputStream mByteArrayOutputStream;

        public ReaderThread(String threadName, AtomicBoolean state) {
            this.isProcessing = state;
        }

        public void run() {
            this.isProcessing.set(true);

            Log.d(getClass().getCanonicalName(), "START");
            this.mByteArrayOutputStream = new ByteArrayOutputStream(2048000);

            int i = 0;
            while (isProcessing.get()) {
                Log.d(getClass().getCanonicalName(), "Iteration: " + i++);
                mByteArrayOutputStream.write(1);
                try {
                    Thread.sleep(1000);
                } catch (InterruptedException e) {
                }
            }

            try {
                mByteArrayOutputStream.reset();
                mByteArrayOutputStream.close();
            } catch (IOException e) {
                e.printStackTrace();
            }

            Log.d(getClass().getCanonicalName(), "STOP");
        }

        public void quit() {
            this.isProcessing.set(false);
        }

        @Override
        protected Integer doInBackground(Integer... params)
        {
            run();
            return null;
        }
    }
}

Here is a comparison of both code samples after they went to and from the home screen a couple of times. You can see a byte[] leak on the first one.

As for why this happens, google recommends that you use a AsyncTask or Service for async operations. I remember one of their videos explicitly advising against Threads. The person making the presentation warned of side effects. I guess this is one of them ? I can clearly see that there is a leak when the activity is brought back to life, but there is no explanation of why said leak would occur (at least on a JVM. I do not know about the internals of the dalvik VM and when it considers threads to have stopped completely). I tried weak-references / nulling the controller / etc and none of those approaches worked.

See this answer for why this leak is reported - https://stackoverflow.com/a/12971464/830964 . It is a false positive.

I was able to get rid of the memory occupied by BAOS without explicitly nulling it out with async tasks. It should work for other variables also. Let me know if that solves it for you.

Answer 4

Threads are immune to GC because they're garbage collection roots. So, it's likely that the JVM is keeping your ReaderThread in memory, along with its allocations for member variables, thus creating the leak.

Nulling out the ByteArrayOutputStream, as you've noted, would make its buffered data (but not the ReaderThread itself) available for GC.

EDIT:

After some sleuthing, we learned that the Android debugger was causing the perceived leak:

The VM guarantees that any object the debugger is aware of is not garbage collected until after the debugger disconnects. This can result in a buildup of objects over time while the debugger is connected. For example, if the debugger sees a running thread, the associated Thread object is not garbage collected even after the thread terminates.

Answer 5

From the Android Debugging page:

The debugger and garbage collector are currently loosely integrated. The VM guarantees that any object the debugger is aware of is not garbage collected until after the debugger disconnects. This can result in a buildup of objects over time while the debugger is connected. For example, if the debugger sees a running thread, the associated Thread object is not garbage collected even after the thread terminates.

That diminishes the value of DDMS heap monitoring don't you think?