JPCERT コーディネーションセンター

LCK02-J. getClass() メソッドが返す Class オブジェクトを同期に使用しない

Object.getClass() メソッドの返り値を使って同期を行うと、プログラムの予期せぬ動作を引き起こす恐れがある。このような同期を実装するクラスのサブクラスは、サブクラスの型に対してロックを行う。サブクラスの Class オブジェクトは、親クラスの Class オブジェクトとはまったく別のものである。

Java言語仕様の§4.3.2「クラスObject」には以下のように規定されている[JLS 2005]。

synchronized 宣言されたクラスメソッドは、そのクラスの Class オブジェクトをロックして同期を行う。

これを読んだプログラマは、getClass() メソッドを使用するサブクラスはスーパークラスの Class オブジェクトを使って同期を行うと解釈するかもしれないが、それは間違いである。プログラマの意図とは異なるかもしれないが、実際には、サブクラスはサブクラス自身の Class オブジェクトに対してロックを行う。したがって、getClass() が返すオブジェクトを使って同期を行ってはならない。

プログラマの実際の意図は、明確に文書化するか、コードに注釈を加えるべきである。このルールに違反しているアクセス可能なスーパークラスのメソッドをサブクラスでオーバーライドしない場合、サブクラスは親クラスのメソッドを継承する。このことは、スーパークラスの固有ロックがサブクラスでも利用できるという誤解を与えるかもしれない。

クラスリテラルを使用して同期を行う場合、信頼できないコードにロックオブジェクトへのアクセスを許してはならない。クラスがパッケージプライベートであれば、他のパッケージにある呼出し元はクラスオブジェクトにアクセスできないので、そのようなクラスの固有ロックを使って同期を行ってもよい。詳細は「LCK00-J. 信頼できないコードから使用されるクラスを同期するにはprivate finalロックオブジェクトを使用する」を参照。

違反コード (getClass()メソッドが返すロックオブジェクト)

以下の違反コード例では、Base クラスの parse() メソッドは、getClass() メソッドが返す Class オブジェクトを使って同期し、日付の解析を行う。Derived クラスは parse() メソッドを継承しているが、この継承されたメソッドの同期は、Base クラスの Class オブジェクトではなく、getClass() メソッドの返り値である Derived クラスの Class オブジェクトに対して行われる。

さらに Derived クラスには、Base クラスの Class オブジェクトに対してロックを行う doSomethingAndParse() メソッドが追加されている。Base クラスの parse() メソッドは Base クラスのオブジェクトに対するロックを常に取得するため、doSomethingAndParse() メソッドも parse() メソッドと同じロックポリシーに従わなくてはならないと考えて、このような間違った実装を行ってしまったのである。結果として、Derived クラスには2つの異なるロックポリシーが混在することになり、スレッドセーフではなくなっている。

class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);

  public Date parse(String str) throws ParseException {
    synchronized (getClass()) {
      return format.parse(str);
    }
  }
}

class Derived extends Base {
  public Date doSomethingAndParse(String str) throws ParseException {
    synchronized (Base.class) {
      // ...
      return format.parse(str);
    }
  }
}
適合コード (クラス名修飾)

以下の適合コードでは、ロックを提供するクラス名(Base)として完全修飾名を使っている。

class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);

  public Date parse(String str) throws ParseException {
    synchronized (Base.class) {
      return format.parse(str);
    }
  }
}

// ...

このコード例では、Derived オブジェクトから呼ばれた場合、常に Base.class オブジェクトを使って同期する。

適合コード (Class.forName()メソッド)

以下の適合コードでは、Class.forName() メソッドを使い、Base クラスの Class オブジェクトを使った同期を行っている。

class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);

  public Date parse(String str) throws ParseException {
    try {
      synchronized (Class.forName("Base")) {
        return format.parse(str);
      }
    } catch (ClassNotFoundException x) {
      // "Base" クラスが見つからない場合のエラー処理
    }
    return null;
  } 
}

// ...

Class.forName() を使用してクラスをロードする際に、信頼できない入力を引数として受け取らないこと。詳しくは「SEC03-J. 信頼できないコードに任意のクラスのロードを許可した後で信頼するクラスをロードしない」を参照。

違反コード (getClass()メソッドが返すロックオブジェクト、内部クラス)

以下の違反コード例では、Base クラスの parse() メソッドの中で、getClass() メソッドが返す Class オブジェクトを使った同期を行っている。また、Base クラスは、内部クラス Helper を持つ。このクラスの doSomethingAndParse() メソッドは getClass() が返す値を使って間違った同期を行っている。

class Base {
  static DateFormat format =
      DateFormat.getDateInstance(DateFormat.MEDIUM);

  public Date parse(String str) throws ParseException {
    synchronized (getClass()) { // Base.class を使った同期を意図
      return format.parse(str);
    }
  }

  public Date doSomething(String str) throws ParseException {
    return new Helper().doSomethingAndParse(str);
  }

  private class Helper {
    public Date doSomethingAndParse(String str) throws ParseException {
      synchronized (getClass()) { // Helper.class を使った同期
        // ...
        return format.parse(str);
      }
    }
  }
}

Helper クラスの getClass() メソッドを呼び出すと、Base クラスの Class オブジェクトではなく、Helper クラスの Class オブジェクトが返される。したがって、Base.parse() メソッドを呼ぶスレッドと Base.doSomething() メソッドを呼ぶスレッドは、それぞれ異なるオブジェクトを使ってロックを行うことになる。内部クラスは、外部クラスの中に記述されていることもあり、内部クラスにおける並行処理に関するエラーは見落とされがちである。コードのレビュー担当者は、2つのクラスのロック方式が同じであると誤解するかもしれない。

適合コード (クラス名修飾)

以下の適合コードでは、parse() メソッドと doSomethingAndParse() メソッドの両方で、Base クラスリテラルを使った同期を行っている。

class Base {
  // ...

  public Date parse(String str) throws ParseException {
    synchronized (Base.class) {
      return format.parse(str);
    }
  }

  private class Helper {
    public Date doSomethingAndParse(String str) throws ParseException {
      synchronized (Base.class) { // Base クラスリテラルを使った同期
        // ...
        return format.parse(str);
      }
    }
  }
}

したがって、BaseHelper クラスはどちらも Base クラスの固有ロックを使って同期を行う。同様に、クラスリテラルの代わりに Class.forname() メソッドを使用してもよい。

リスク評価

getClass() メソッドが返す Class オブジェクトを使用した同期は、予期せぬプログラムの動作を引き起こす場合がある。

ルール 深刻度 可能性 修正コスト 優先度 レベル
LCK02-J P8 L2
参考文献
[API 2006]  
[Findbugs 2008]  
[Pugh 2008] Synchronization
[Miller 2009] Locking
翻訳元

これは以下のページを翻訳したものです。

LCK02-J. Do not synchronize on the class object returned by getClass() (revision 68)

Top へ

Topへ
最新情報(RSSメーリングリストTwitter