Java : synchronizedの多いコードは危険信号

2023年05月23日 2021年06月03日

Tweet

スレッドは適切に使うと、プログラムのパフォーマンス向上にとても有効です。
しかし、スレッドを使い、同期が必要だからと安易にsynchronizedを使っていないでしょうか。

synchronizedの多いコードは、それだけスレッド要因のやっかいなバグが発生するリスクが高くなります。
スレッドは使っても、できるだけsynchronizedは少なくなるような設計を心がけましょう。

この記事の対象読者：スレッドと同期(synchronized)をある程度使ったことのあるかた

要点

最初に要点をまとめてしまいます。

• スレッドの使用は慎重に。
  • スレッドを使うとどうしてもコードは複雑になるので、必要最低限にしましょう。
• スレッドを使っても、できるだけsynchronizedは使わない設計を心がけましょう。
  • synchronizedで待たされたらスレッドのメリットも小さくなります。
  • 手法の１つとして、シングルスレッドモデルがあります。
  • volatileキーワードも同様です。使わない設計を心がけましょう。
• 自分のコードに不安があれば、スレッドに慣れているかたにコードレビューをお願いしてみましょう。

スレッドのおさらい

それではさっそくコードを見ていきましょう。

public class Counter {
    private int value;

    public void increment() {
        value++;
    }

    public int getValue() {
        return value;
    }
}

Counterクラスは、incrementメソッドを呼ぶとvalueを+1していく単純なクラスになります。

final var counter = new Counter();
final var executor = Executors.newWorkStealingPool(2);

try {
    final Runnable task = () -> {
        IntStream.range(0, 100).forEach(i -> {
            counter.increment();
        });
    };

    executor.submit(task);
    executor.submit(task);

} finally {
    executor.shutdown();
}
executor.awaitTermination(10, TimeUnit.SECONDS);

System.out.println(counter.getValue());

スレッドを２つ作り、それぞれのスレッドでcounterに対してincrementを100回ずつ呼び出します。
最後にcounterの値を表示させます。

さて、結果はどうなるか予想できますでしょうか？
スレッドを抜きにして考えれば200になりそうですが…

実行結果

158

もう１回実行してみましょう。

183

200にならないどころか、毎回結果が変わります。

今回は単純な例なのでよいですが、これが巨大で複雑なコードの中で起こったら…
考えるだけで胃が痛くなります。

なぜ問題が起こるのか？

厳密なスレッドの仕様は、非常にややこしく直感的なものではありません…
本気で理解しようとすると、メモリモデルの理解からはじまり大変です。

ここでは分かりやすさ優先で、本当に単純な、簡略化した問題として解説します。
厳密な仕様には即さないかもしれませんが、ご了承ください。

先ほどのCounterクラスを少し分かりやすく変更します。

public class Counter {
    private int value;

    public void increment() {
        int a = value + 1; // あ行
        value = a; // い行
    }

    public int getValue() {
        return value;
    }
}

それ以外のところは変わりません。

public void main() throws InterruptedException {
    final var counter = new Counter();
    final var executor = Executors.newWorkStealingPool(2);

    ...

ExecutorServiceで使う２つのサブスレッドを、スレッドA、スレッドBとします。

incrementメソッドを２回呼び出して、結果としてvalue = 2になる例です。
期待する動作ですね。

しかし、incrementの呼び出しがスレッドAとスレッドBで重なると、期待した結果になりません。

incrementメソッドを２回呼び出したのに、結果としてvalue = 1になります。

このように、複数のスレッドから、共有の変数(Counterのvalue)にアクセスすると、競合して期待した結果にならないことがあります。
この問題のやっかいなところは、期待した動作になるときもあれば、ならないときもある、というところです。

大きなプログラムでこの手の問題が起こると、たいてい再現率が非常に低いです。

問題が発生したから解析して！
↓
詳細なログを出力するようにして、問題を再現させて調査しよう。
↓
再現しない…

というのもよくある話です。

synchronizedのおさらい

さきほどのコードを期待どおりに動作させる方法の１つとしてsynchronizedがあります。

public class Counter {
    private int value;

    public synchronized void increment() {
        int a = value + 1; // あ行
        value = a; // い行
    }

    public int getValue() {
        return value;
    }
}

incrementメソッドにsynchronizedキーワードをつけます。

これでincrementメソッドが丸ごとロックされます。
１つのCounterインスタンスに対して、incrementメソッドの呼び出しは最大１つのスレッドのみしか処理されません。

もし２つのスレッドから同時にincrementメソッドが呼び出されたら、どちらかのスレッドのみ(例えばスレッドA)がincrementメソッドの処理を行います。スレッドBは、スレッドAのincrementメソッドの呼び出しが終わるまで待機します。

あれ？と思ったかたもいらっしゃるかもしれません。
getValueにもsynchronizedが必要なのではないか、と。

今回の例だけで言えば、スレッドA, BをshutdownしてからgetValueしているので問題は発生しません。
しかし、例えばスレッドCからgetValueするケースでは、問題が発生するかもしれません。

一貫性を持たせるためにgetValueにもsynchronizedをつけた方がよいでしょう。

まとめると…

• 共有する変数(Counterのvalue)をサブスレッドから読み書きする場合はsynchronizedが必要です。
• ローカル変数は各スレッドで作成されるので同期は不要です。

synchronizedのデメリット

パフォーマンス低下

複数のスレッドで並列に処理しているのに、頻繁にsynchronizedで同期してしまっては、せっかくの並列処理のメリットが小さくなります。
単純に待たされる分だけパフォーマンスが低下します。

synchronizedが多く必要となる場合は、設計を見直すことも検討してみたほうがよいかもしれません。

テストが難しい

本当に同期(synchronized)されているか？ということをテストするのは難しいです。

手動で同期のテストをするのは現実的ではありません。
やるとするならば、Unit Testで何百回、何千回とまわす、という感じでしょうか。(それでも確実とは言えませんが…)

コーディングするのに気をつかう

テストで確認するのが難しいので、基本的にはコーディングのときに気をつけることになります。

これは意外とプログラマにとって負担になります。
いろいろと慎重になるので、結果としてコーディングの効率は落ちるでしょう。

あとは単純に難しいです。
どこにsynchronizedをつければいいのか？というのを理解するには、一定以上のスキルが必要です。

もし自分の書いたコードに自信がないときは、スレッドに慣れているかたにコードレビューをお願いしてみるのをおすすめします。

デッドロック

synchronizedが多いとデッドロックのリスクも高くなります。

デッドロックとは、２つのロックを２つのスレッドからお互いにロックしあおうとすると、永久にロックが解除されない状態のことをいいます。
言葉だと分かりづらいので、コードで見てみましょう。

public class X {

    private Y y;

    public void setY(Y y) {
        this.y = y;
    }

    public synchronized void funcY() throws InterruptedException {
        System.out.println("X.funcY -- start-- ");
        Thread.sleep(1000);

        y.yyy();
        System.out.println("X.funcY -- end -- ");
    }

    public synchronized void xxx() {
        System.out.println("X.xxx");
    }
}

public class Y {

    private X x;

    public void setX(X x) {
        this.x = x;
    }

    public synchronized void funcX() throws InterruptedException {
        System.out.println("Y.funcX -- start -- ");
        Thread.sleep(1000);

        x.xxx();
        System.out.println("Y.funcX -- end --");
    }

    public synchronized void yyy() {
        System.out.println("Y.yyy");
    }
}

２つのクラスを用意します。
クラスXとクラスYは互いに参照しあっています。

一般的に、このように依存が循環するのはよくないと言われています。
そして、同期(synchronized)を使うと簡単にデッドロックが発生します。

funcXとfuncYではデッドロックが発生しやすいように１秒のスリープを入れています。
理論上はスリープがなくても発生します。

final var executorService = Executors.newWorkStealingPool(2);
try {
    final var x = new X();
    final var y = new Y();

    x.setY(y);
    y.setX(x);

    executorService.submit(() -> {
        try {
            x.funcY();
        } catch (InterruptedException e) {
            System.out.println("InterruptedException!");
        }
    });

    executorService.submit(() -> {
        try {
            y.funcX();
        } catch (InterruptedException e) {
            System.out.println("InterruptedException!");
        }
    });

} finally {
    executorService.shutdown();
}

// タスクが完了するまで10秒待機します。
final var term = executorService.awaitTermination(10, TimeUnit.SECONDS);
System.out.println("term : " + term);

// 結果
// ↓
//X.funcY -- start--
//Y.funcX -- start --
//term : false

X.funcYとY.funcXをそれぞれ別のスレッドから呼び出します。
これでデッドロックが発生しました。

スリープは１秒しかいれてないので、デッドロックが発生していなければ、それぞれのタスクは１秒後に正常に終了するはずです。
しかし、１０秒たってもタスクは完了しませんでした。

シーケンス図も見てみましょう。

伝わりましたでしょうか。
互いにロックをしようとして、すでにロック済みなので、どちらもロック解除を待ち続けてしまいます。
このロックは永久に解除されることはありません。

プログラムがデッドロックを起こすと、ユーザにはアプリがフリーズしたように見えるでしょう。
フリーズは致命的な問題の１つです。

synchronizedを使わないようにするには？

シングルスレッドモデル

共有する変数にアクセスするスレッドを、ある特定の１つのスレッドに限定させます。
１つのスレッドからしか読み書きされないので、競合は発生せず、同期も不要という考えです。
下図ではメインスレッドがそれにあたります。

ちょっと無理やり感があるかもですが、Counterクラスをシングルスレッドモデルとして使ってみましょう。

public static class Counter {
    private int value;

    public void increment() {
        value++;
    }

    public int getValue() {
        return value;
    }
}

final var counter = new Counter();

final var mainExecutor = Executors.newSingleThreadExecutor();
try {

    final var subExecutor = Executors.newWorkStealingPool(2);
    try {
        final Runnable task = () -> {
            IntStream.range(0, 100).forEach(i -> {
                mainExecutor.submit(() -> {
                    counter.increment();
                });
            });
        };

        subExecutor.submit(task);
        subExecutor.submit(task);

    } finally {
        subExecutor.shutdown();
    }
    subExecutor.awaitTermination(10, TimeUnit.SECONDS);

    mainExecutor.submit(() -> {
        // 200
        System.out.println(counter.getValue());
    });

} finally {
    mainExecutor.shutdown();
}

mainExecutor.awaitTermination(10, TimeUnit.SECONDS);

Counterのincrementメソッドは、シングルスレッドであるmainExecutorでのみ実行します。
Counterはsynchronizedしていませんが、毎回期待どおりに結果 = 200 となります。

もう１つ例をご紹介します。

一般的にスレッドが必要になるときは、なにか重い処理をスレッドに任せたい、というときだと思います。
よくあるのは、ファイルの読み書きをサブスレッドで行う、というケースです。

サブスレッドでファイルの読み書きをしている間は、GUIでプログレスバーを表示させる、というのはよくありますね。

そちらもコード例で見てみましょう。

public class Task implements Runnable {
    private final Path path;
    private final Consumer<String> callback;

    public Task(Path path, Consumer<String> callback) {
        this.path = path;
        this.callback = callback;
    }

    @Override
    public void run() {
        try {
            callback.accept(Files.readString(path));
        } catch (IOException e) {
            e.printStackTrace();
        }
    }
}

Taskクラスは、読み込むファイル先を指定するPathと、結果を通知するためのConsumerであるcallbackを受け取ります。
ちょっと例外へのフォローがあまいのはご容赦ください。

ここで重要なのは、タスクに渡すパラメータを不変オブジェクトにすることです。
標準APIのPathは不変オブジェクトです。

不変オブジェクトは、その名のとおりオブジェクトの内容が変わることはありません。
そのため、どのスレッドからどのタイミングでアクセスしても値は同じになります。

共有する変数と違い、サブスレッドからアクセスしても問題ありません。

Consumerは厳密には不変オブジェクトではありません。
しかし、タスクの処理が完了することを通知したいので渡しています。

// resultは結果集計用です。
final var result = new ArrayList<String>();

final var mainExecutor = Executors.newSingleThreadExecutor();
try {

    final var subExecutor = Executors.newWorkStealingPool();
    try {
        final Consumer<String> callback = (s) -> {
            // サブタスクからの結果をメインスレッドで処理させます。
            mainExecutor.submit(() -> result.add(s));
        };

        final var path1 = Path.of("R:", "java-work", "aaa.txt");
        subExecutor.submit(new Task(path1, callback));

        final var path2 = Path.of("R:", "java-work", "bbb.txt");
        subExecutor.submit(new Task(path2, callback));

    } finally {
        subExecutor.shutdown();
    }
    subExecutor.awaitTermination(10, TimeUnit.SECONDS);

    mainExecutor.submit(() -> {
        // [AAA, BBB]
        System.out.println(result);
    });

} finally {
    mainExecutor.shutdown();
}
mainExecutor.awaitTermination(10, TimeUnit.SECONDS);

ファイルの"aaa.txt"と"bbb.txt"を２つのスレッドで並列に読み込みます。
それぞれのファイルの中身は"AAA"と"BBB"です。

そして、読み込みが完了したら、結果をメインスレッド上で追加(result.add)しています。

これで重い処理(ファイルからの読み込み)はサブスレッド上で並列に処理させ、共有する変数のアクセス(result.add)はメインスレッドで行うことができました。
synchronizedは不要です。

シングルスレッドモデルは、GUIフレームワーク(SwingやJavaFXなど)では、よく導入されている設計手法です。

例えば、JavaFXでは、GUIのパーツにアクセスするには、ある特定の１つのスレッドからアクセスする必要があります。
それ以外のスレッドからアクセスすると例外が発生します。

このように、１つのスレッドからのみにアクセスを制限することによって、フレームワーク全体でsynchronizedを省略できる、というわけです。
結果として、コードはシンプルになり、同期によるパフォーマンス低下もなくなります。

まとめると…

• 共有する変数にアクセスするスレッドは１つに限定して、なるべくsynchronizedが必要ない設計にしましょう。
  • SwingやJavaFXを使っている場合は、GUIのメインスレッドがそれに相当することになると思います。
• 不変オブジェクトはマルチスレッドで安全です。サブスレッドで処理するタスクのパラメータには不変オブジェクトを渡しましょう。

補足

不変オブジェクト

不変オブジェクトとは、コンストラクタでインスタンスが生成されたら、それ以降、内容が変更されないことが保証されたオブジェクトです。

標準APIでも不変オブジェクトとして定義されているものが多くあります。
代表的なAPIは文字列(String)です。

公式API仕様書にも明記されていますね。
他には、PathやLocalDateTime、プリミティブ型なんかも不変です。

実際に、不変クラスのコード例を見てみましょう。

public final class ImmutableSample {
    private final int a;
    private final String b;

    public ImmutableSample(int a, String b) {
        this.a = a;
        this.b = b;
    }

    public int getA() {
        return a;
    }

    public String getB() {
        return b;
    }
}

フィールドはすべてfinalをつけて上書きを禁止します。
Stringは不変オブジェクトなので、this.bの内容が書き換わることはありません。

このように、コンストラクタでオブジェクトが生成されたら、それ以降オブジェクトの内容が変更できないようにします。

不変なので、どのスレッドからどのタイミングで読み込んでも、同じ結果となります。
つまりマルチスレッドで安全なオブジェクトです。

マルチスレッドに限らず、不変オブジェクトで設計することはメリットになることが多いです。
不変オブジェクトについては別途記事にしましたので、そちらもご参照ください。

関連記事：不変オブジェクト(イミュータブル) とは

標準APIの古いAPIには不変オブジェクトではないものがありました。
例えば日時系のAPIです。

• Date (Java SE 15 & JDK 15)
• Calendar (Java SE 15 & JDK 15)

上記のクラスは不変オブジェクトではありません。
これらは、Java 8で新しいAPIに置き換えられました。

• LocalDateTime (日時・タイムゾーンなし)
• ZonedDateTime (日時・タイムゾーンあり)
• Instant (時点)

などです。
新しい日時系のAPIはすべて不変オブジェクトです。

このように、標準APIの変更を見ても、不変オブジェクトが有用であることが分かります。

関連記事

• 不変オブジェクト(イミュータブル) とは
• スレッドで発生した例外は忘れずに処理しよう
• Thread.sleep の代わりに TimeUnit.sleep を使う
• Object.waitの注意点

• API 使用例
  • BlockingQueue (ブロッキング・キュー)
    • ArrayBlockingQueue (ブロッキング・配列キュー)
    • BlockingDeque (ブロッキング・両端キュー)
    • LinkedBlockingDeque (ブロッキング・リンク両端キュー)
    • LinkedBlockingQueue (ブロッキング・リンクキュー)
    • PriorityBlockingQueue (ブロッキング・優先度付きキュー)
  • Callable
  • CancellationException
  • ConcurrentHashMap.KeySetView (並列処理用セット)
  • ConcurrentLinkedDeque (並列処理用・両端キュー)
  • ConcurrentLinkedQueue (並列処理用キュー)
  • ConcurrentMap (並列処理用マップ)
    • ConcurrentHashMap (並列処理用マップ)
    • ConcurrentSkipListMap (並列処理用マップ)
  • ConcurrentModificationException (並列処理例外)
  • ConcurrentSkipListSet (並列処理用セット)
  • Condition (同期)
  • CopyOnWriteArrayList (並列処理用リスト)
  • CopyOnWriteArraySet (並列処理用セット)
  • CountDownLatch (同期)
  • CyclicBarrier (同期)
  • Exchanger (同期)
  • Executor
  • ExecutorService
  • Executors
  • Future
  • Future.State
  • FutureTask
  • InterruptedException (割込み例外)
  • Lock (同期)
    • ReentrantLock (同期)
  • Object (オブジェクト)
  • Runnable
  • Semaphore (セマフォ)
  • Thread (スレッド)
  • ThreadGroup
  • ThreadLocal
    • InheritableThreadLocal
  • TimeUnit