synchronizedブロックに使用されているStaticObjectが変更される場合、実質synchronizedされていない

synchronizedブロックに使用されている変数が書き換わる場合それはスレッドセーフではありません。(まあ当然です)

例えばこれはNG

class TargetDate {
    private static String  staticCal = "書き換えられるかな?";
    public void set(String s){
        staticCal = s;
    }
    public void echo(){
        synchronized (staticCal) {
            for (int i = 1; i <= 100; i++) {
                System.out.println(i  + "回目 [" + staticCal + "]");
            }
        }
    }
}

setメソッド内でstaticCalの値を変更しているときにシンクロしていないのでEchoの呼び出し側スレッドが影響を受けてしまいます。
Echoメソッドとしてはこのメソッド実行中は他のスレッドにstaticCalにアクセスしてほしくないのですがロックの鍵が変わってしまっているため他のスレッドが入り込めてしまいます。
試しにこのEchoとSetをマルチスレッドで呼び出してみるとEchoのループの20回目あたりで値が変更されてしまいます。

1回目 [書き換えられるかな?]
2回目 [書き換えられるかな?]
3回目 [書き換えられるかな?]
4回目 [書き換えられるかな?]
5回目 [書き換えられるかな?]
6回目 [書き換えられるかな?]
7回目 [書き換えられるかな?]
8回目 [書き換えられるかな?]
9回目 [書き換えられるかな?]
10回目 [書き換えられるかな?]
11回目 [書き換えられるかな?]
12回目 [書き換えられるかな?]
13回目 [書き換えられるかな?]
14回目 [書き換えられるかな?]
15回目 [書き換えられるかな?]
16回目 [書き換えられるかな?]
17回目 [書き換えられるかな?]
18回目 [書き換えてやるぜ!!!]
19回目 [書き換えてやるぜ!!!]
20回目 [書き換えてやるぜ!!!]

だめな対策

全部の呼び出し箇所をすべてシンクロするので対応可能と考えがちですが、シンクロしたとしてもロック対象オブジェクトが変更される場合
ロックの取得元が異なるためロックになりません。

class UnSefeTargetDate {
    private static String  staticCal = "ロックだよ";
    public void setAndEcho(String s){
        synchronized (staticCal) {
            // staticCalへのアクセスは同期化されているので以下は順番に実行されるはず。
            staticCal = s;
            for ( int i = 0 ; i < 5; i++ ) {
                System.out.println(staticCal + " ["+ i + "]");
                try {
                    Thread.sleep(20);
                } catch (InterruptedException e) {}
            }
        }
    }
}
class SyncThread extends Thread {
    public void run() {
        new UnSefeTargetDate().setAndEcho("1");
    }
}
class ReplaceThread extends Thread {
    public void run() {
        new UnSefeTargetDate().setAndEcho("2");    
    }
}
    public static void main(String[] args) throws Exception {
        new SyncThread().start();
        new ReplaceThread().start();
    }

こんなメソッドはロックを取れておらず試しに多重で動かしてみると以下のようにstaticCalは書き換えられるし多重実行されてしまいます。

1 [0]
2 [0]
2 [1]
2 [1]
2 [2]
2 [2]
2 [3]
2 [3]
2 [4]
2 [4]

正しい対策

private final static なロック専用のオブジェクトを使いましょう。
staticじゃなかったりthisを使うとインスタンスが異なる場合ロックにならないので注意しましょう。
(シングルトンなりインスタンスが同じなら大丈夫ですが。)インスタンス複数存在するクラスのStaticなものを同期化したい目的で使うならロックObjectもStaticである必要があります。

class SefeTargetDate {
    private static String  staticCal = "書き換えられないのだ";
    private final static Object lock = new Object();
    public void setAndEcho(String s){
        synchronized (lock) {
            staticCal = s;
            for ( int i = 0 ; i < 5; i++ ) {
                System.out.println(staticCal + " ["+ i + "]");
                try {
                    Thread.sleep(20);
                } catch (InterruptedException e) {}
            }
        }
    }
}

これなら安心。

1 [0]
1 [1]
1 [2]
1 [3]
1 [4]
2 [0]
2 [1]
2 [2]
2 [3]
2 [4]

いやあ、マルチスレッドのバグ探しは難しいね。
かなり難儀な感じだったけど見つけられてよかった。これで他の事象の説明もついたし、今回は両方当てたし。よかったよかった



てか、製品として売るならこんぐらいのバグ潰せよ!!!