失敗は一時の恥

パッケージソフト開発をしているプログラマが気の赴くままに何かを投稿するブログ.

マルチスレッド下で SimpleDateFormat をおかしくしてみる

概要

JavaSimpleDateFormat クラスはスレッドセーフでないことで知られていると思いますが,実際にマルチスレッド下で使ったらどうなるのかを試してみました.

実際に試すと,エラーになることもなくメチャクチャな値が得られるという悲惨な結果となりました.

背景

最近,職場のコードで SimpleDateFormat をマルチスレッドで使用している箇所が見つかり騒ぎになったので,実際にどういう挙動をするのか調べてみようと思った次第です.

「見つかり騒ぎになった」というのも,お客さんから製品の怪しい動作を指摘されて調査した結果発覚したという経緯になっていて,他のお客さんも含めたその後の対応もあり色々と辛い出来事でした.

実験

SimpleDateFormat をマルチスレッドで使うこんな感じのコードを書いてみました:

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

public class BreakDateFormat {
    static SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ");

    public static void main(String[] args) throws ParseException, InterruptedException {
        final int numThreads = 10;
        final int numLoops = 10;
        final String dateString = "2001-07-04T12:08:56.235-0700";
        final Set<Date> parsedDates = Collections.synchronizedSet(new HashSet<>());

        ExecutorService executorService = Executors.newFixedThreadPool(numThreads);
        for (int i = 0; i < numThreads; i++) {
            executorService.submit(new Runnable() {
                @Override
                public void run() {
                    try {
                        for (int j = 0; j < numLoops; j++) {
                            parsedDates.add(dateFormat.parse(dateString));
                        }
                    } catch (ParseException e) {
                        e.printStackTrace();
                    }
                }
            });
        }
        executorService.shutdown();
        executorService.awaitTermination(5, TimeUnit.SECONDS);

        System.out.println("size=" + parsedDates.size());
        System.out.println("values:");
        for (Date date : parsedDates) {
            System.out.println(dateFormat.format(date));
        }
    }
}

やっていることのポイントは

  • static な SimpleDateFormat のインスタンスを作る: static SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ");
  • executorService に上記のインスタンスを使って parsedDates.add(dateFormat.parse(dateString)); を実行するタスクを放り込んでマルチスレッドで動作させる

というところです.

どんな値が得られるのかは最終的に,Set parsedDates の中身を標準出力に出します.

結果

10 スレッドで 10 回ずつタスクを回してみて,手元では次のような結果が得られました.

size=6
values:
1669500-12-05T04:08:56.235+0900
0001-07-05T04:41:21.235+0900
2008-04-05T04:08:56.235+0900
2001-07-05T04:08:56.235+0900
2000-12-05T04:08:56.235+0900
2059-08-05T04:08:56.235+0900

かなりぶっ飛んだ値が得られました.

こんな値に従って動作したら,システムもさぞ問題のある挙動をすることでしょう……

この問題を防ぐにはどうすればよかったか

コードレビューをきちんとしていればさすがに誰かは気づいたのではないかというのと,ツールとして SpotBugs なんかを使えば,static な SimpleDateFormat インスタンスの使用箇所を検出してくれたりもして,防げたのではないかという印象です. 実際に,今回書いた不具合再現プログラムに対しても指摘してくれています.

f:id:snobutaka:20181029084521p:plain
spotbugs_find_static_date_format

所感

やってはいけないことというのを学んだり,やってはいけないことをコードレビューで見つけるなり,個人の知識や開発プロセスとして何かしらできることはあったはずだという無念な気持ちが湧きました.

特に今回のレベルの問題は SpotBugs で機械的に検知できるものでもあったので,後で見つかってから部門をまたがって協議したり緊急対応パッチを作ったりユーザ全体へ通達を出したりなんかするコストを考えると,仕組みで防げるものは潰していけるようにツールなりなんなり導入するのは十分投資として見合うものだな〜と思ったり.