22

I just read a blogpost by Romain Guy on how to avoid memory leaks in Android.

In the article he gives this example:

private static Drawable sBackground;

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

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

  if (sBackground == null) {
    sBackground = getDrawable(R.drawable.large_bitmap);
  }
  label.setBackgroundDrawable(sBackground);

  setContentView(label);
}

Romain said:

This example is one of the simplest cases of leaking the Context.

My question is, how do you modify it correctly?

Just like this?

TextView label = new TextView(Context.getApplicationContext());

I tested both ways and the results are the same. I can't locate the difference. And I think that this is more correct than the Application context. Because this is a reference to Activity, that is to say, the TextView belongs to that Activity.

Could someone give me an explanation for this?

JDJ
  • 4,298
  • 3
  • 25
  • 44
Judy
  • 1,772
  • 6
  • 27
  • 48

4 Answers4

17

The actual problem with that code isn't the context passed to create the drawable, but private static Drawable sBackground; The static Drawable is created with the Activity as the context, so in THAT case, there's a static reference to a Drawable that references the Activity, and that's why there's a leak. As long as that reference exists, the Activity will be kept in memory, leaking all of its views.

So it's the Drawable which should be created using the application context, not the TextView. Creating the TextView with "this" is perfectly fine.

edit : Actually, that might not make a big difference, the problem is that once the drawable is binded to a view, there's a reference to the view, which references the activity. So you need to "unbind" the drawable when you exit the activity.

Gregory
  • 4,384
  • 1
  • 25
  • 21
  • 1
    it's the Drawable which should be created using the application context-->you think it should is `sBackground = getApplicationContext().getResources().getDrawable(R.drawable.icon);`Is that right? – Judy Jul 04 '11 at 07:23
  • @Judy Yes, that should be better. – Gregory Jul 04 '11 at 07:47
  • @Judy Actually, no, that shouldn't make a difference, the problem is that when you bind the drawable to a view, it gets a reference to it, which holds a reference to the activity. So you actually need to cleanly unbind the drawable from the view. – Gregory Jul 04 '11 at 07:52
  • 1
    "unbind the drawable from the view"-->How to unbind the drawable from the view? just like this code: ` @Override protected void onDestroy() { // TODO Auto-generated method stub label.setBackgroundDrawable(null); super.onDestroy(); }' Please tell me how to implement it.Thank you – Judy Jul 05 '11 at 01:09
  • Call setCallback(null) on your drawable. – Gregory Jul 05 '11 at 04:09
1

I'm not sure if Romain had updated his blog entry since you read it, but he's pretty clear on how to avoid the leaks, even pointing you to an example in the Android OS. Note that I fixed the broken link in Romain's blog entry via archive.org.

This example is one of the simplest cases of leaking the Context and you can see how we worked around it in the Home screen's source code (look for the unbindDrawables() method) by setting the stored drawables' callbacks to null when the activity is destroyed. Interestingly enough, there are cases where you can create a chain of leaked contexts, and they are bad. They make you run out of memory rather quickly.

There are two easy ways to avoid context-related memory leaks. The most obvious one is to avoid escaping the context outside of its own scope. The example above showed the case of a static reference but inner classes and their implicit reference to the outer class can be equally dangerous. The second solution is to use the Application context. This context will live as long as your application is alive and does not depend on the activities life cycle. If you plan on keeping long-lived objects that need a context, remember the application object. You can obtain it easily by calling Context.getApplicationContext() or Activity.getApplication().

In summary, to avoid context-related memory leaks, remember the following:

  • Do not keep long-lived references to a context-activity (a reference to an activity should have the same life cycle as the activity itself)
  • Try using the context-application instead of a context-activity
  • Avoid non-static inner classes in an activity if you don't control their life cycle, use a static inner class and make a weak reference to the activity inside. The solution to this issue is to use a static inner class with a WeakReference to the outer class, as done in ViewRoot and its W inner class for instance
  • A garbage collector is not an insurance against memory leaks
Jeff Axelrod
  • 27,676
  • 31
  • 147
  • 246
0

Memory leaks at that code mostly happen when you rotate your screen (that is, changing the orientation state) so your activity was destroyed and created again for the new orientation. There's a lot of explanation about memory leaks.

You can take a look at one of the Google I/O 2011 video about Memory Management here. In the video, you can also use the memory management tools like Memory Analyzer available to download here.

Kristiono Setyadi
  • 5,635
  • 1
  • 19
  • 29
0

I don't know if you are having any trouble with this in your app, but I have created a drop in solution that fixes all the android memory leak issues with standard android classes: http://code.google.com/p/android/issues/detail?id=8488#c51

public abstract class BetterActivity extends Activity
{
  @Override
  protected void onResume()
  {
    System.gc();
    super.onResume();
  }

  @Override
  protected void onPause()
  {
    super.onPause();
    System.gc();
  }

  @Override
  public void setContentView(int layoutResID)
  {
    ViewGroup mainView = (ViewGroup)
      LayoutInflater.from(this).inflate(layoutResID, null);

    setContentView(mainView);
  }

  @Override
  public void setContentView(View view)
  {
    super.setContentView(view);

    m_contentView = (ViewGroup)view;
  }

  @Override
  public void setContentView(View view, LayoutParams params)
  {
    super.setContentView(view, params);

    m_contentView = (ViewGroup)view;
  }

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

    // Fixes android memory  issue 8488 :
    // http://code.google.com/p/android/issues/detail?id=8488
    nullViewDrawablesRecursive(m_contentView);

    m_contentView = null;
    System.gc();
  }

  private void nullViewDrawablesRecursive(View view)
  {
    if(view != null)
    {
      try
      {
        ViewGroup viewGroup = (ViewGroup)view;

        int childCount = viewGroup.getChildCount();
        for(int index = 0; index < childCount; index++)
        {
          View child = viewGroup.getChildAt(index);
          nullViewDrawablesRecursive(child);
        }
      }
      catch(Exception e)
      {          
      }

      nullViewDrawable(view);
    }    
  }

  private void nullViewDrawable(View view)
  {
    try
    {
      view.setBackgroundDrawable(null);
    }
    catch(Exception e)
    {          
    }

    try
    {
      ImageView imageView = (ImageView)view;
      imageView.setImageDrawable(null);
      imageView.setBackgroundDrawable(null);
    }
    catch(Exception e)
    {          
    }
  }

  // The top level content view.
  private ViewGroup m_contentView = null;
}
swinefeaster
  • 2,525
  • 3
  • 30
  • 48
  • Does that actually work ? I thought there were just some delay between when the activity is destroyed and when it is garbage collected. – ben Feb 24 '13 at 10:04