On Memory Leaks in Java and in Android.

Just because it’s a garbage collected language doesn’t mean you can’t leak memory or run out of it. Especially on Android where you get so little to begin with.

Now of course sometimes the answer is that you just need more memory. If your program is a Java command line program to load the entire road map of the United States to do some network algorithms, you probably need more than the default JVM configurations give you.

Sometimes it’s not even a full-on leak, but a large chunk of memory isn’t being released in time as a consequence of some holder object that isn’t being released in time.

There are some tools that can help. With Android, you can use DDMS to get an idea what’s going on, and you can even dump a snapshot of the heap by using the Dump HPROF File option. (You can also programmatically capture uncaught exceptions on startup of your application or activity and dump an hprof file within the exception handler like so:

public void onCreate(Bundle savedInstanceState)
{
...
    Thread.setDefaultUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler()
    {
        @Override
        public void uncaughtException(Thread thread, Throwable ex)
        {
            try {
                File f = new File(Environment.getExternalStorageDirectory(),"error.hprof");
                String path = f.getAbsolutePath();
                Debug.dumpHprofData(path);
                Log.d("error", "HREF dumped to " + path);
            }
            catch (IOException e) {
                Log.d("error","Huh?",e);
            }
        }
    });
...
}

Of course once you have an .hprof file from Android you have to convert it to something that can be used by an application such as the Eclipse Memory Analyzer tool using the hprof-conv command line application included as part of the Android SDK; there is more information on how to do this and how to use the MAT tool here: Attacking memory problems on Android.

One place where I’ve been running into issues is with a clever little bit of code which loads images from a separate thread from a remote resource, and puts them into a custom view that replaces the ImageView class. This little bit of code creates a background thread which is used to talk to a remote server to download images; once the image is loaded, a callback causes the custom view to redraw itself with the correct contents. A snippet of that code is below:

/*  Cache.java
 *
 *  Created on May 15, 2011 by William Edward Woody
 */

package com.chaosinmotion.android.utils;

import java.io.File;
import java.io.FileOutputStream;
import java.io.InputStream;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.Map.Entry;

import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.DefaultHttpClient;

import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.os.Handler;

public class Cache
{
    /**
     * Our callback interface
     */
    public interface Callback
    {
        void loaded(String url, Bitmap bitmap);
        void failure(String url, Throwable th);
    }
    
    /**
     * Item in the queue which is waiting to be processed by our network thread(s)
     */
    private static class QueueItem
    {
        String url;
        Callback callback;
        
        QueueItem(String u, Callback c)
        {
            url = u;
            callback = c;
        }
    }
    
    /// The handler to thread to the UI thread
    private Handler fHandler;
    /// The event queue
    private LinkedList<QueueItem> fQueue;
    /// The global cache object, which will be created by the class loader on load.
    /// Because this is normally called from our UI objects, this means our Handler
    /// will be created on our UI thread
    public static Cache gCache = new Cache();
    
    /**
     * Internal runnable for our background loader thread
     */
    private class NetworkThread implements Runnable
    {
        public void run()
        {
            // Start HTTP Client
            HttpClient httpClient = new DefaultHttpClient();
            
            for (;;) {
                /*
                 * Dequeue next request
                 */
                QueueItem q;

                synchronized(fQueue) {
                    while (fQueue.isEmpty()) {
                        try {
                            fQueue.wait();
                        }
                        catch (InterruptedException e) {
                        }
                        break;
                    }

                    /*
                     * Get the next item
                     */
                    q = fQueue.removeLast();
                }
                
                /*
                 * Read the network
                 */
                
                try {
                    /*
                     * Set up the request and get the response
                     */
                    HttpGet get = new HttpGet(q.url);
                    HttpResponse response = httpClient.execute(get);
                    HttpEntity entity = response.getEntity();
                    
                    /*
                     * Get the bitmap from the URL response
                     */
                    InputStream is = entity.getContent();
                    final Bitmap bmap = BitmapFactory.decodeStream(is);
                    is.close();

                    entity.consumeContent();
                    
                    /*
                     * Send notification indicating we loaded the image on the
                     * main UI thread
                     */
                    final QueueItem qq = q;
                    fHandler.post(new Runnable() {
                        public void run()
                        {
                            qq.callback.loaded(qq.url,bmap);
                        }
                    });
                }
                catch (final Throwable ex) {
                    final QueueItem qq = q;
                    fHandler.post(new Runnable() {
                        public void run()
                        {
                            qq.callback.failure(qq.url,ex);
                        }
                    });
                }
            }
            
//            httpClient.getConnectionManager().shutdown();
        }
    }
    
    /**
     * Start up this object
     */
    private Cache()
    {
        fHandler = new Handler();
        fQueue = new LinkedList();
        Thread th = new Thread(new NetworkThread());
        th.setDaemon(true);
        th.start();
    }
    
    /**
     * Get the singleton cache object
     */
    public static Cache get()
    {
        return gCache;
    }
    
    /**
     * Get the image from the remote service. This will call the callback once the
     * image has been loaded
     * @param url
     * @param callback
     */
    public void getImage(String url, Callback callback)
    {
        synchronized(fQueue) {
            fQueue.addFirst(new QueueItem(url,callback));
            fQueue.notify();
        }
    }
}

Now what this does is rather simple: we have a queue of items which are put into a linked list, and our background thread loads those items, one at a time. Once the item is loaded, we call our callback so the image can then be handled by whatever is using the service to load images from a network connection.

Of course we can make this far more sophisticated; we can save the loaded files to a cache, we can collapse multiple requests for the same image so we don’t try to load it repeatedly. We can also make the management of the threads more sophisticated by creating a thread group of multiple threads all handling network loading.

We can then use this with a custom view class to draw the image, drawing a temporary image showing the real image hasn’t been loaded yet:

/*  RemoteImageView.java
 *
 *  Created on May 15, 2011 by William Edward Woody
 */

package com.chaosinmotion.android.utils;

import android.content.Context;
import android.graphics.Bitmap;
import android.graphics.Canvas;
import android.graphics.Color;
import android.graphics.Paint;
import android.view.View;

public class RemoteImageView extends View
{
    private Paint fPaint;
    private Bitmap fBitmap;
    private String fURL;

    public RemoteImageView(Context context)
    {
        super(context);
        // TODO Auto-generated constructor stub
    }
    
    public void setImageURL(String url)
    {
        fBitmap = null;
        fURL = url;
        
        Cache.get().getImage(fURL, new Cache.Callback() {
            public void loaded(String url, Bitmap bitmap)
            {
                fBitmap = bitmap;
                invalidate();
            }

            public void failure(String url, Throwable th)
            {
                // Ignoring for now. Could display broken link image
            }
        });
    }

    @Override
    protected void onDraw(Canvas canvas)
    {
        if (fPaint == null) fPaint = new Paint();
        
        canvas.drawColor(Color.BLACK);
        if (fBitmap == null) return;        // could display "not loaded" image
        canvas.drawBitmap(fBitmap, 0, 0, fPaint);
    }
}

This is a very simple example of our using the Cache object to load images from a background thread. We can make this far more sophisticated; we can (for example) display a “loading” image and a “image link broken” image. We can also alter the reported size during onMeasure to return the size of the bitmap, or we can center the displayed bitmap or scale the bitmap to fit. But at it’s core, we have a simple mechanism for displaying the loaded image in our system.

Can you spot the leak?

I didn’t, at first.

Here’s a hint: Avoiding Memory Leaks

Here’s another: the RemoteImageView, being a child of the View class, holds a reference to it’s parent, and up the line until we get to the top level activity, which holds a reference to–well–just about everything.

No?

Okay, here goes.

So when we call:

        Cache.get().getImage(fURL, new Cache.Callback() { ... });

The anonymous inner class we create when we create our callback holds a reference to the RemoteImageView. And that inner class doesn’t go away until after the image is loaded. So if we have a few dozen of these and a very slow connection, the user switches from one activity to another–and we can’t let the activity go, because we’re still waiting for the images to load and be copied into the image view.

So while it’s not exactly a memory leak, the class can’t be let go of, nor can all the associated resources, until our connection completes or times out. In theory it’s not a leak, exactly, because eventually the memory will be released–but it won’t be released soon enough for our purposes. And so we crash.

So how do we fix this?

Well, we need to add two things. First, we need to somehow disassociate our view from the anonymous inner class so that, when our view no longer exists, the callback class no longer holds a reference to the view. That way, the activity can be reclaimed by the garbage collector even though our callback continues to exist. Second, we can remove the unprocessed callbacks so they don’t make a network call to load an image that is no longer needed.

To do the first, we change our anonymous inner class to a static class (that way it doesn’t hold a virtual reference to ‘this’), and explicitly pass a pointer to our outer class to it, one that can then be removed:

/*  RemoteImageView.java
 *
 *  Created on May 15, 2011 by William Edward Woody
 */

package com.chaosinmotion.android.utils;

import android.content.Context;
import android.graphics.Bitmap;
import android.graphics.Canvas;
import android.graphics.Color;
import android.graphics.Paint;
import android.view.View;

public class RemoteImageView extends View
{
    private Paint fPaint;
    private Bitmap fBitmap;
    private String fURL;
    private OurCallback fCallback;
    
    public RemoteImageView(Context context)
    {
        super(context);
        // TODO Auto-generated constructor stub
    }
    

    private static class OurCallback implements Cache.Callback
    {
        private RemoteImageView pThis;
        
        OurCallback(RemoteImageView r)
        {
            pThis = r;
        }
        
        public void loaded(String url, Bitmap bitmap)
        {
            if (pThis != null) {
                pThis.fBitmap = bitmap;
                pThis.invalidate();
                pThis.fCallback = null; // our callback ended; remove reference
            }
        }

        public void failure(String url, Throwable th)
        {
            // Ignoring for now. Could display broken link image
            if (pThis != null) {
                pThis.fCallback = null; // our callback ended; remove reference
            }
        }
    }

    public void setImageURL(String url)
    {
        fBitmap = null;
        fURL = url;
        
        fCallback = new OurCallback(this);
        Cache.get().getImage(fURL, fCallback);
    }

    @Override
    protected void onDraw(Canvas canvas)
    {
        if (fPaint == null) fPaint = new Paint();
        
        canvas.drawColor(Color.BLACK);
        if (fBitmap == null) return;        // could display "not loaded" image
        canvas.drawBitmap(fBitmap, 0, 0, fPaint);
    }

    @Override
    protected void onDetachedFromWindow()
    {
        // Detach us from our callback
        if (fCallback != null) fCallback.pThis = null;
        
        super.onDetachedFromWindow();
    }
}

The two biggest changes is to create a new static OurCallback class which holds a reference to the view being acted on. We then hold a reference to the callback that is zeroed out when the callback completes, either on failure or on success. Then on the onDetachedFromWindow callback, if we have a request outstanding (because fCallback is not null), we detach the view from the callback. Note that because all the calls in the callback are done on the UI thread we don’t need to synchronize access.

This will now detach the view from the callback when the view goes away, so the activity that contains the view can be reclaimed by the memory manager.

Our second change is to remove the request from the queue, so we don’t use unnecessary resources. While not strictly necessary for memory management purposes, it helps our network performance. The change here is to explicitly remove our callback from the queue.

First, we change our onDetachedFromWindow() call to remove us (by callback) from the cache:

    @Override
    protected void onDetachedFromWindow()
    {
        // Detach us from our callback
        if (fCallback != null) {
            fCallback.pThis = null;
            Cache.get().removeCallback(fCallback);
        }
        
        super.onDetachedFromWindow();
    }

Second, we add a method to the cache to look for all instances of requests with the same callback, and delete the request from the queue. If it isn’t in the queue, it’s probably because the request is now being acted upon by our networking thread. (If we were particularly clever we could signal our networking thread to stop the network request, but I’m not going to do that here.)

So our method added to the Cache is:

    /**
     * Remove from the queue all requests with the specified callback. Done when the
     * result is no longer needed because the view is going away.
     * @param callback
     */
    public void removeCallback(Callback callback)
    {
        synchronized(fQueue) {
            Iterator iter = fQueue.iterator();
            while (iter.hasNext()) {
                QueueItem i = iter.next();
                if (i.callback == callback) {
                    iter.remove();
                }
            }
        }
    }

This iterates through the queue, removing entries that match the callback.

I’ve noted this on my list of things not to forget because this (and variations of this) comes up, with holding references to Android View objects in a thread that can survive the destruction of an activity.

The basic model is when the view goes away (which we can detect with a callback to onDetachedFromWindow), to disassociate the callback from the view and (preferably) to kill the background thread so the view object (and the activity associated with that view) can be garbage collected in a timely fashion.

This entry was posted in Algorithms, Android, Java. Bookmark the permalink.

3 Responses to On Memory Leaks in Java and in Android.

  1. joebowbeer says:

    Nice writeup. Thanks.

    I think your leak-free implementation could be improved by using a (final) WeakReference in the Callback instead of a (non-final) strong reference.

    This may even eliminate the need for explicit onDetachedFromWindow handling – though the detach handler could always clear the weak refs…

    See post by Romain Guy

    http://groups.google.com/group/android-developers/msg/959f98f16a038694

    Though the “static” qualifier is unfortunately missing from the inner class declaration. An oversight I believe.

  2. I have a habit of shying away from using WeakReference in general because there doesn’t seem to be an obvious way to indicate in which order the GC should release objects. I’m sure you probably could do it with a clever use of the ReferenceQueue class, but I’m not quite smart enough to figure that out. So in general I tend to not think of using WeakReferences in general.

    That said, you’re right: if I put in a weak reference to RemoteImageView pThis inside RemoteImageView.OurCallback, I could forgo having to release the reference; the record in the queue wouldn’t hold onto the (now deleted) View, keeping it in memory. Though I’d still need the onDetachedFromWindow call to call into removeCallback in the Cache object; no point in fetching an image view via an expensive network call for a view that no longer exists.

    Though the “static” qualifier is unfortunately missing from the inner class declaration. An oversight I believe.

    Are you talking about the Cache.NetworkThread inner class? (RemoteImageView.OurCallback is declared static.)

    I can’t; the Cache.NetworkThread class uses the outer Cache.fQueue field.

  3. joebowbeer says:

    The “static” qualifier I was referring to is in Romain’s template code.

    His Inner class should be declared static, else the WeakReference will be moot.

Leave a Reply