Understanding memory leaks in Android application

Question

I found the article "Avoiding memory leaks", where it is said that the following code:

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);
}

is not a good idea, since:

When the screen orientation changes the system will, by default, destroy the current activity and create a new one while preserving its state. In doing so, Android will reload the application's UI from the resources.

So the above code:

...leaks the first activity created upon the first screen orientation change. When a Drawable is attached to a view, the view is set as a callback on the drawable. In the code snippet above, this means the drawable has a reference to the TextView which itself has a reference to the activity (the Context) which in turns has references to pretty much anything (depending on your code.)

But, when screen orientation changes, the method setBackgroundDrawable(Drawable background) is called, which in turn calls:

background.setCallback(this);

The method Drawable.setCallback() is definied in the following way:

public final void setCallback(Callback cb) {
    mCallback = new WeakReference<Callback>(cb);
}

So, now background should release the old reference to the previous TextView, and a new reference to the new TextView should be created.

So, it seems like changing screen orientation leaks a reference only until the activity is newly created.

Where am I going wrong ?

Answer 1

You are absolutely right. However, there is one subtle point: the article is from 2009. Back then, the implementation of setCallback was different:

Android <= 2.3.7:

public final void setCallback(Callback cb) {
    mCallback = cb;
}

Android >= 4.0.1:

public final void setCallback(Callback cb) {
    mCallback = new WeakReference<Callback>(cb);
}

_{Grepcode shows no source code of intermediate versions, this is the only diff I could quickly find.}

---

So, again, you're absolutely right in this specific case (if you're targeting >14 that is). However, it is still very important to really think about what is actually happening when you keep a static reference to such items (like you did). There are numerous cases where you certainly could be leaking the Context.