シングルトンインスタンスの static 宣言は一番最後に書く

この前、後輩からきかれてちょっとびっくりしたこと。
一見するとただのシングルトンパターンのクラスが、なぜか動かないと言われました。

public class Sample {
	private static final Sample instance = new Sample();
	private static Map<String, String> map = null;
	
	private Sample(){
		map = Collections.synchronizedMap(new HashMap<String, String>());
	}
	
	public static Sample getInstance(){
		return instance;
	}
	
	public void set(String key, String value){
		map.put(key, value);	// NullPointerException!
	}
}

不思議に思ってデバッグしたら、setメソッドで map が null になっており、そこでNullPointerException が出ました。


たしかに宣言時に null を代入してますが・・・。
でも、ちゃんとコンストラクタで初期化しているように見えます。


・・・クラス変数の初期化は確か上からの順のハズ?*1
ということで、さらにデバッグしてみたら、これが原因でした。


どういうことかというと、

  • instance 変数を初期化
  • map を null で初期化

という順序・・・。


とりあえず、宣言時の null 初期化をやめたらちゃんと動きました。
でも、これはちょっと不思議。
変数宣言時の初期化が、その変数が使われたこあとに行われています。



この辺の動きは、気をつけないと変なはまり方をしそうですね・・・。
他にも、こういう例はやってしまいそうです(たしか Effective Java にも載っていたような…)。

public class Sample {
	private static final Sample instance = new Sample();
	private static final Map<String, String> map = Collections.synchronizedMap(new HashMap<String, String>());
	
	private Sample(){
		map.put("Test", "NG");	// ここで NullPointerException!
	}
	
	public static Sample getInstance(){
		return instance;
	}
}


教訓:シングルトンインスタンスの static 宣言は一番最後に書く



2011/7/18 追記:
コメントをいただきました。

ToriKen
『static変数の初期化を、コンストラクタなどのインスタンスメソッドで行っていることが、根本的な原因だと思います。
解決法としては、「mapをインスタンス変数にする」もしくは「mapの初期化をstaticイニシャライザで行う」の二択になると思います。』

ご指摘いただいたように「mapをインスタンス変数にする」に修正したところ、順番に関係なくちゃんと動くようになりました!
これを書いていたときに、static 変数とインスタンス変数を区別せずに考えてしまっていました・・・。
(ToriKenさん、ありがとうございました)


修正後のソース:

public class Sample {
	private static final Sample instance = new Sample();
	private Map<String, String> map = null;
	
	(以下略)

*1:念のため、Java言語仕様第3版で確認したところ、12.4.1「静的初期化子とクラス変数初期化子はテキスト順に実行され、・・・」となっていたので、これであってます。ただし、8.3.2.1「些細な点であるが、static変数がfinalとして宣言されており、かつコンパイル時の定数値で初期化される場合、それは実行時に最初に初期化される。(中略)こういった変数は「定数」であり、どのようなプログラムをもってしてもデフォルトの初期値が取得されることはない」とも。なので、クラスのfinal定数 -> クラス変数初期化が正確な順序です。