Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Proper onDestroy() / How to avoid memory leaks

I read a number of articles on how to avoid memory-leaks in Android, but i'm still not quite sure if I got it right.

  1. My Application consists of a single Activity.
  2. I don't have any private or static members in that Activity, all code is started from within onCreate().
  3. In have some self-contained static classes whose static instances sometimes hold references to a Context or Views. In my onDestroy() method, I set all of these instances to null.
  4. I recycle all of my Bitmaps.

Q1: Is that enough?

What confuses me is the classic example of a no-go you can find on the net (http://www.curious-creature.org/2008/12/18/avoid-memory-leaks-on-android/):

@Override
protected void onCreate(Bundle state) {
  super.onCreate(state);

  TextView label = new TextView(this);
  label.setText("Leaks are bad");

  setContentView(label);
}

I thought that, as soon as onCreate finishes, label goes out of scope and is GCed.

Q2: How can this create a memory-leak?

My Activity basically looks like this:

@Override
public void onCreate(final Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);

    /* Statics */
    AssetUtils.initIndex(this);
    BitmapLoader.startInstance(this);

    /* frame */
    ViewGroup frame = (ViewGroup) getLayoutInflater().inflate(R.layout.frame, null);
    this.setContentView(frame);

    /* create controller */
    Controller controller = new Controller(frame, getLayoutInflater());

    /* START */
    controller.start();
}

@Override
public void onDestroy() {
    super.onStop();

    /* Statics */
    AssetUtils.destroyInstance();
    BitmapLoader.destroyInstance();
}

Inside Controller I occasionally retrieve a Context using View#getContext() to pass it to manually created Views and the like. It's never stored statically somewhere, only in member-variables of classes which all go back to Controller.

Q3: Is there something I overlooked?

like image 465
Fabian Zeindl Avatar asked Feb 07 '13 19:02

Fabian Zeindl


1 Answers

Q1. You've taken this out of context (no joke intended :)

If you see the original article, the leak actually occurs in the next fragment of code where the Bitmap field is introduced. Roman then clearly explains why it leaks. The code you have shown will NOT leak.

http://www.curious-creature.org/2008/12/18/avoid-memory-leaks-on-android/

Q2. Use Activity context only where there is no other choice and NEVER allow a reference to it in something with a scope greater than the scope of the Activity it references. The code you've shown doesn't leak as far as I can see since nothing has a context reference with a scope greater than your Activity. Do you suspect that it does?

Q3. When using Bitmaps, static references or not, I am in the habit of unbinding Bitamps from Views in onPause() (remember onDestroy() is not guaranteed but it's kind of irrelevant as if you're being destroyed, your process is killed so GC isn't a concern). The linked article also explains how to do this. I have made it a template pattern for any of my Activities handling Bitmaps.

[EDIT]

Sorry, I just checked. The article does not show how to unbind. Here's the pattern I use:

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

        unbindDrawables(findViewById(R.id.mainLayout));
        System.gc();
}


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

        unbindDrawables(findViewById(R.id.mainLayout));
        System.gc();
}

private void unbindDrawables(View view)
{
        if (view.getBackground() != null)
        {
                view.getBackground().setCallback(null);
        }
        if (view instanceof ViewGroup && !(view instanceof AdapterView))
        {
                for (int i = 0; i < ((ViewGroup) view).getChildCount(); i++)
                {
                        unbindDrawables(((ViewGroup) view).getChildAt(i));
                }
                ((ViewGroup) view).removeAllViews();
        }
}

mainLayout is the root view of the Activity layout.

I include onDestroy() since I might manually finish() my Activity.

Note. Calling system.gc() is a complex subject and you might want to omit it. There are some good discussions on here why it might not be a good thing to do, principally concerned with performance. However, in my view, when an activity is being destroyed, hinting that now is a good time to perform GC can do no harm. The inefficiency of calling it unnecessarily will be lost in the overheads of destroying an activity.

like image 175
Simon Avatar answered Sep 30 '22 17:09

Simon