Androidのソースコードレビュー(メモリリーク)

有限であるモバイル端末のメモリを無駄に消費するアプリは悪だ。そんなアプリのソースコードレビュー担当者の目は節穴だ。

と言われないためか否かは知らないけど、Android Developers - Avoiding Memory Leaksでは、Androidアプリを作成する上で、メモリリークが発生するパターンとその対処法を色々と紹介している。(英語)

要点のみを自分が見直し易いようにまとめつつ、説明の足りてない「非staticな内部クラスが駄目な件」について書いてみる。

そもそもメモリリークするん？

Java実行環境下では、ガベージコレクタによるメモリ管理が行われているため大抵の場合は意識せずともメモリをいい感じに回収してくれる。だが、ガベージコレクタの動作を妨げるようなコードを書いてしまうと、いつまで経っても解放されるべきメモリが解放されなくなる。これが今回のお題であるJavaにおけるメモリリークの定義である。

---

問題 1

view plainprint?

1. @Override  
2. protected void onCreate(Bundle state)  
3. {  
4.   super.onCreate(state);  
5.     
6.   TextView label = new TextView(this);  
7.   label.setText("Leaks are bad");  
8.     
9.   setContentView(label);  
10. }  

@Override
protected void onCreate(Bundle state)
{
  super.onCreate(state);
  
  TextView label = new TextView(this);
  label.setText("Leaks are bad");
  
  setContentView(label);
}

何が問題か

labelのコンストラクタでthis(=Activityインスタンス)を渡しているため、labelがActivityの参照を保持している。このためActivityを破棄しても上手いことGCの対象となってくれない。（labelによる参照が残っている）

対処法 1

そもそも各Viewオブジェクトのコンストラクタで渡しているコンテキスト(上記ではthis(=Activityインスタンス))を、アプリケーションコンテキストに変更する。その場合、コードは下記のようになるはず。

view plainprint?

1. @Override  
2. protected void onCreate(Bundle state)  
3. {  
4.   super.onCreate(state);  
5.     
6.   TextView label = new TextView(getApplicationContext());  
7.   label.setText("Leaks are bad");  
8.     
9.   setContentView(label);  
10. }  

@Override
protected void onCreate(Bundle state)
{
  super.onCreate(state);
  
  TextView label = new TextView(getApplicationContext());
  label.setText("Leaks are bad");
  
  setContentView(label);
}

対処法 2

レイアウトはXMLで定義する。その場合、コードは下記のようになるはず。

view plainprint?

1. @Override  
2. protected void onCreate(Bundle state)  
3. {  
4.   super.onCreate(state);  
5.   setContentView(R.layout.main);  
6. }  

@Override
protected void onCreate(Bundle state)
{
  super.onCreate(state);
  setContentView(R.layout.main);
}

---

問題 2

view plainprint?

1. private static Drawable sBackground;  
2.     
3. @Override  
4. protected void onCreate(Bundle state)  
5. {  
6.   super.onCreate(state);  
7.     
8.   TextView label = new TextView(getApplicationContext());  
9.   label.setText("Leaks are bad");  
10.     
11.   if (sBackground == null)  
12.   {  
13.     sBackground = getDrawable(R.drawable.large_bitmap);  
14.   }  
15.   label.setBackgroundDrawable(sBackground);  
16.     
17.   setContentView(label);  
18. }  

private static Drawable sBackground;
  
@Override
protected void onCreate(Bundle state)
{
  super.onCreate(state);
  
  TextView label = new TextView(getApplicationContext());
  label.setText("Leaks are bad");
  
  if (sBackground == null)
  {
    sBackground = getDrawable(R.drawable.large_bitmap);
  }
  label.setBackgroundDrawable(sBackground);
  
  setContentView(label);
}

何が問題か

Drawableは、Viewにアタッチされた際(上記で言うところの label.setBackgroundDrawable(sBackground))に、アタッチ先Viewの参照をcallbackとして保持する。このため、アタッチ先のView(もしくはそのViewが参照している全て)は、当該Drawableインスタンスが破棄されない限りGCの対象となってくれない。
上記コードの場合、Drawableはstaticな変数であるため最悪な感じ。

対処法

Drawableインスタンスがcallbackとして保持している参照を消す。その場合、コードは下記のようになるはず。

view plainprint?

1. private static Drawable sBackground;  
2.     
3. @Override  
4. protected void onCreate(Bundle state)  
5. {  
6.   super.onCreate(state);  
7.     
8.   TextView label = new TextView(getApplicationContext());  
9.   label.setText("Leaks are bad");  
10.     
11.   if (sBackground == null)  
12.   {  
13.     sBackground = getDrawable(R.drawable.large_bitmap);  
14.   }  
15.   label.setBackgroundDrawable(sBackground);  
16.   
17.   // callback参照を削除  
18.   sBackground.setCallback(null);  
19.     
20.   setContentView(label);  
21. }  

private static Drawable sBackground;
  
@Override
protected void onCreate(Bundle state)
{
  super.onCreate(state);
  
  TextView label = new TextView(getApplicationContext());
  label.setText("Leaks are bad");
  
  if (sBackground == null)
  {
    sBackground = getDrawable(R.drawable.large_bitmap);
  }
  label.setBackgroundDrawable(sBackground);

  // callback参照を削除
  sBackground.setCallback(null);
  
  setContentView(label);
}

---

問題 3

view plainprint?

1. class MyActivity extends Activity  
2. {  
3.   class InnnerClass  
4.   {  
5.     private Activity parent;  
6.       
7.     InnerClass(Activity aParent)  
8.     {  
9.       parent = aParent;  
10.     }  
11.   }  
12. }  

class MyActivity extends Activity
{
  class InnnerClass
  {
    private Activity parent;
    
    InnerClass(Activity aParent)
    {
      parent = aParent;
    }
  }
}

何が問題か

この短いコードに2つも問題点が含まれているが、いずれも親クラスへの参照を保持しているのが問題である。

• 非staticな内部クラスである(参考:Effective Java)
  
• 親クラス(MyActivity)への強参照を持っている
  

対処法

対処法はお決まりの方法であり「非staticな内部クラスは作らない」「ライフサイクルの異なるインスタンス参照は弱参照とする」である。

view plainprint?

1. class MyActivity extends Activity  
2. {  
3.   // 内部クラスは原則としてstaticとすべき  
4.   static class InnnerClass  
5.   {  
6.     private WeakReference<Activity> parent;  
7.       
8.     InnerClass(Activity aParent)  
9.     {  
10.       // 弱参照とする  
11.       parent = new WeakReference<Activity>(aParent);  
12.     }  
13.   }  
14. }  

class MyActivity extends Activity
{
  // 内部クラスは原則としてstaticとすべき
  static class InnnerClass
  {
    private WeakReference<Activity> parent;
    
    InnerClass(Activity aParent)
    {
      // 弱参照とする
      parent = new WeakReference<Activity>(aParent);
    }
  }
}

参考

• Avoiding Memory Leaks - Android Developers
  
• Androidでのメモリリーク回避 - textdrop
  
• [Android] Memory Leaks - OGAKI SOFT
  
• Re: static vs non-static inner classes - Android Developers
  
• 非staticな内部クラスが持つ暗黙的な参照 - ellerの日記