Android : Static Fields and Memory Leaks

Question

I've been studying up on best practices for preventing Context/Activity memory leaks when creating views, and I can't seem to find a definite answer on what is or is not allowed when it comes to static fields in classes.

Let's say I have a code of this form:

public class MyOuterClass extends Activity{
   private MyInnerClass;
   MyInnerClass = (MyInnerClass) findViewById(<XML call here>);
   MyInnerClass.myXInt = 3;

   // onCreate(), onResume(), etc.

   public static class MyInnerClass extends SurfaceView implements Runnable{
      // Safe variables?
      private static int myXInt, myYInt;
      private static boolean myBoolean;
      // Potentially safe?
      private static Canvas myCanvas;
      // Definitely bad.
      private static Context myContext;

      public MyInnerClass(Context context){
         myContext = context;        // This is bad.
      }
   }
}

I am slightly confused on what the JVM actually considers the ClassLoader for MyInnerClass. Technically, since it is a SurfaceView object, it seems like the static variables should always exist once the application has instantiated MyInnerClass one time (which happens when the View is first inflated), and then remain there until the application itself is terminated. If that is the case, what prevents Bitmaps and Canvas objects from remaining open as well and filling up the heap?

The only statement I ever see repeated over and over is that you can't leak static Context like I have shown in the constructor, but it never goes beyond that. Is that really the only thing you can't do?

Solution

In Java/Android a static variable or constant will not be garbage collected. It just stays there once the class that holds it is loaded via a class loader. The class loader is afaik always the same for all classes inside your app and its the one that has static references to all your classes (to e.g. MyInnerClass.class). Since the class loader does not go away your classes won't do that either since they are referenced & therefore not garbage collectable.

Like in your example

public class SomeClass extends SurfaceView {
  private static Context myContext;

  public MyInnerClass(Context context){
     myContext = context;        // This is bad.
  }
}

That is indeed bad. Even if no reference to SomeClass exists (e.g. the Activity that showed your custom SurfaceView has ended) the static reference to the Context (and any other static variable / constant in SomeClass will remain. You can consider all of them leaked since it is not possible to garbage collect that Context etc. If you have a regular variable reference something then once the instance that contains that variable has no more references to it the whole instance including its references to other things can and will be garbage collected. Java can even handle circular references fine.

For constants you want that to happen and it is usually not bad since the amount of constants and the amount of memory they occupy is not large. Also constants don't (should not) reference other instances that take up large amounts of memory like Context or Bitmap.

Besides the possibility to create memory leaks through static variables you may also create problems if you don't want to have only a single thing for all instances at the same time. For example if you save the Bitmap of your SurfaceView in a static variable you can't have two different images. Even if the two SurfaceViews are not displayed at the same time you could run into problems since each new instance will probably overwrite the old image and if you go back to the other SurfaceView you unexpectedly show the wrong image. I am almost sure you don't want to use static here.

The fact that your inner class is a static class does not mean that you have to use static variables - it just means that it behaves more like a static method since it can't use the instance variables (the ones that are not static) in your class.

To avoid memory leaks you simply should not use static variables at all. There is no need to use them unless you do special stuff (e.g. counting instances of a class). Constants are fine.

Comments

[zapl]

your Canvas etc does not need to be static. That way it would indeed stay in the heap forever

[SeaNick]

If that is the case then what prevents constants (i.e. - private static final int MY_CONSTANT), from also holding any class that extends activity (and its context) open?

[SeaNick]

Thanks. I guess the overall reasoning for wanting statics was because conceptually, most View objects only ever need one instance, since they usually represent a specific screen or object on the screen. It seems much more efficient code-wise to make them static, but I understand the drawback there. The only thing that bothers me about leaving the variables at the total mercy of the GC is that I have no control over when it will need to free memory, and from everything I've seen in my app, it's always during a bitmap operation, which I'm afraid will turn into jittery frame rates.

[zapl]

You don't need to have something static to keep it alive and reuse it. It will exists as long as you have a reference to it. And using static will usually lead to more memory consumption if you forget to null all your static references appropriately. + View instances will usually be recreated if you rotate the screen for example

[SeaNick]

I keep the screen orientation locked, but that's a very good point. Thanks again!

[suitianshi]

good explanation, I was using weakReference to the context to avoid memory leak