Wicket 1.3.6がリリースされて、気になるバグFIXがあったり

はじめに

 Wicket 1.3.6で、気になるバグFIXを見つけたのでメモです。
 とはいえ「あ。対応入ったんだ。そういう直し方かー」という程度だったりもするのですが。
 今回のエントリは、[WICKET-2191] WebApplication is not thread-safe - ASF JIRAから、id:sekomが「へー」と思ったことを抽出したものです。

どのバグか

 Release Notes - Wicket - Version 1.3.6に記載されているWICKET-2191が今回ネタにするバグです。
 バグのタイトルは下記の通り。

WICKET-2191 - WebApplication is not thread-safe

バグの概要

 バグの内容は、[WICKET-2191] WebApplication is not thread-safe - ASF JIRAに記載されています。
 簡単に説明をすると、org.apache.wicket.protocol.http.WebApplication.addBufferedResponse()実行時に、 java.util.ConcurrentModificationExceptionが送出されることがあるというもの。
 同時接続数が2以上ならば、発生する可能性はあります。現実的には、同時接続数2ではまず発生しないですが。

 org.apache.wicket.protocol.http.WebApplication.addBufferedResponse()は、Wicketが内部で使用しているメソッドです。
 呼び出されるタイミングは、わたしの記憶によれば、画面レンダリング後です。

 報告されている事象では、bufferedResponses.put()で例外が出ることがあるとのこと。
 例外が出るか出ないか、putの実装次第な気もしますので「あるベンダーのあるバージョンのVMでは発生するけど、他のVMでは発生しない」という可能性もあるかもしれません。
 以下にバグFIX前のソースを抜粋します。

    private final Map bufferedResponses = new HashMap();
//中略
    final void addBufferedResponse(String sessionId, String bufferId,
        BufferedHttpServletResponse renderedResponse)
    {
        Map responsesPerSession = (Map)bufferedResponses.get(sessionId);
        if (responsesPerSession == null)
        {
            responsesPerSession = new MostRecentlyUsedMap(4);
            bufferedResponses.put(sessionId, responsesPerSession);
        }
        responsesPerSession.put(bufferId, renderedResponse);
    }

バグの修正方法

 下記の実装にWicket1.3.6では変わるそうです。

import org.apache.wicket.util.concurrent.ConcurrentHashMap;
//中略
    private final Map bufferedResponses = new ConcurrentHashMap();
//中略

    final void addBufferedResponse(String sessionId, String bufferId,
        BufferedHttpServletResponse renderedResponse)
    {
        Map responsesPerSession = (Map)bufferedResponses.get(sessionId);
        if (responsesPerSession == null)
        {
            responsesPerSession = Collections.synchronizedMap(new MostRecentlyUsedMap(4));
            Object removed = bufferedResponses.put(sessionId, responsesPerSession);
            if (removed != null)
            {
                responsesPerSession.putAll((Map)removed);
            }
        }
        responsesPerSession.put(bufferId, renderedResponse);
    }

 org.apache.wicket.util.concurrent.ConcurrentHashMapの詳細な仕様は知りませんが、教科書的な直し方とは違いますね。
 教科書的な直し方ならば、synchronizedキーワードがどこかに出てくるはずです。

 直し方を見て「??」と私は思ったわけですが、[WICKET-2191] WebApplication is not thread-safe - ASF JIRAに直し方の意図が記載されています。
 私の英語力では正確なところは読み取れませんが、憶測を交えつつまとめますと、以下のような感じ。

  • synchronizedを使うとパフォーマンスが代償になる。concurrent HashMapが完全に死ぬ(パフォーマンス的な意味で)。
  • 実際の所、大半のスレッド同士は関係ないのでconcurrent HashMapが有効に使えるシチュエーションである。
  • 二つのタブまたはウインドウから、全く同時刻に新しいセッションの要求がきたら、キーが被ってbufferedResponses.putが実行されてしまうことがある。そのときは、生成した新しいresponsesPerSessionに、先にbufferedResponsesへputされていたresponsesPerSessionをコピーする。

 「bufferedResponses.putの実行が被らないようにする」ではなく、「bufferedResponses.putの実行が被ったときに考える」が教科書的な直し方と異なるポイントの一つのようです。

さいごに

 バグの修正方法の項に書いた「直し方の意図」は憶測による文章の補完が入ってますので、信用しないでください。
 ぱっと見ただけで、正確な理解が難しそうだと感じたのに、なぜがんばって読んだんだ、わたしは……。