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

VNA03-J. アトミックなメソッドをまとめた呼び出しがアトミックであると仮定しない

一貫したロックポリシーを採用することで、複数のスレッドが同時に共有データにアクセスしたり変更したりできないようにすることができる。2つ以上の操作を1つのアトミックな操作として実行する場合、固有ロックを使ってメソッドを同期するか java.util.concurrent ユーティリティを使用して、一貫したロックポリシーを実装しなくてはならない。そのようなポリシーが存在しないと、競合状態が発生する。

各々はアトミックであることが保証されている複数の操作から構成された1つの処理は、アトミックであると考えがちであるが、これは間違いである。同様に、スレッドセーフなCollectionクラスさえ使えば、更なる同期を行わなくてもコレクションに関する不変条件は保持されると、間違って考えるかもしれない。スレッドセーフなクラスは、クラスの個々のメソッドのアトミック性しか保証できない。個々にアトミックなメソッド呼出しをまとめて行う場合、その処理全体を同期する必要がある。

たとえば、単一のメソッドで、特定の人物のレコードをHashtable上で検索し、かつその人物の給与支払簿情報を更新することが、標準のスレッドセーフAPIでは実現できない場合について考えてみよう。そのような場合、2つのメソッド呼出しをアトミックに実行しなくてはならない。

列挙やイテレータを使用する場合にも、該当するコレクションオブジェクトを明示的に同期するか(クライアントサイドロック)、private final宣言したロックオブジェクトを用いる必要がある。

共有変数に対する複合操作もアトミックではない。詳細は「VNA02-J. 共有変数への複合操作のアトミック性を確保する」を参照。

VNA04-J. メソッドチェーン呼出しのアトミック性を確保する」はこのルールの特殊ケースについて解説している。

違反コード (AtomicReference クラス)

以下の違反コードでは、BigIntegerオブジェクトをスレッドセーフなAtomicReferenceオブジェクトでラップしている。

final class Adder {
  private final AtomicReference<BigInteger> first;
  private final AtomicReference<BigInteger> second;

  public Adder(BigInteger f, BigInteger s) {
    first  = new AtomicReference<BigInteger>(f);
    second = new AtomicReference<BigInteger>(s);
  }

  public void update(BigInteger f, BigInteger s) { // スレッドセーフではないメソッド
    first.set(f);
    second.set(s);
  }

  public BigInteger add() { // スレッドセーフではないメソッド
    return first.get().add(second.get());
  }
}

AtomicReferenceはアトミックに更新することができるオブジェクト参照である。しかし、複数の AtomicReference を組み合わせた操作はアトミックではない。この違反コード例では、あるスレッドが add() を呼び出している最中に他のスレッドが update() を呼び出すかもしれない。その場合、add() メソッドは、first の新しい値を更新前の second の値に加算してしまい、結果として間違った計算結果が得られるかもしれない。

適合コード (メソッドの同期)

以下の適合コードでは、アトミック性を保証するためにupdate()メソッドとadd()メソッドをsynchronized 修飾している。

final class Adder {
  // ...
  private final AtomicReference<BigInteger> first;
  private final AtomicReference<BigInteger> second;

  public Adder(BigInteger f, BigInteger s) {
    first  = new AtomicReference<BigInteger>(f);
    second = new AtomicReference<BigInteger>(s);
  }



  public synchronized void update(BigInteger f, BigInteger s){
    first.set(f);
    second.set(s);
  }

  public synchronized BigInteger add() {
    return first.get().add(second.get());
  }
}
違反コード (synchronizedList()メソッド)

以下の違反コードではスレッドセーフではないjava.util.ArrayList<E>コレクションを使用しているが、同期処理のために Collections.synchronizedList を、ArrayList のラッパーとして用いている。ArrayList に対する繰り返し処理にイテレータではなく配列を用いることで、ConcurrentModificationExceptionを回避している。

final class IPHolder {
  private final List<InetAddress> ips = 
      Collections.synchronizedList(new ArrayList<InetAddress>());

  public void addAndPrintIPAddresses(InetAddress address) {
    ips.add(address);
    InetAddress[] addressCopy = 
        (InetAddress[]) ips.toArray(new InetAddress[0]);
    // 配列addressCopyを通して繰り返し処理を行う ...
  }
}

add()およびtoArray()メソッドは、それぞれアトミックである。しかし、それらが連続して呼ばれた場合(たとえば addAndPrintIPAddresses() メソッドの中で行われているように)、「組み合わされた」操作がアトミックである保証はない。addAndPrintIPAddresses()メソッドには競合状態が存在し、あるスレッドがリストへ追加し終わる前に他のスレッドがリストを変更できてしまう。addressCopy配列は想定よりも多くのIPアドレスを持つ結果となるかもしれない。

適合コード (Synchronized ブロック)

リストへのアクセスを同期させることで、競合状態を取り除くことができる。以下の適合コードでは、配列リストへのすべての参照をsynchronizedブロックの中にまとめている。

final class IPHolder {
  private final List<InetAddress> ips = 
      Collections.synchronizedList(new ArrayList<InetAddress>());

  public void addAndPrintIPAddresses(InetAddress address) {
    synchronized (ips) {
      ips.add(address);
      InetAddress[] addressCopy = 
          (InetAddress[]) ips.toArray(new InetAddress[0]);
      // 配列addressCopyを通して繰り返し処理を行う ...
    }
  }
}

この手法は「クライアントサイドロック」とも呼ばれる[Goetz 2006]。その理由は、他のクラスからアクセス可能なオブジェクトの固有ロックをクラスが持つからである。クライアントサイドロックを用いるのが常に適切であるとは限らない。詳細は「LCK11-J. 一貫したロック方式を定めていないクラスを使用する場合、クライアントサイドロックを行わない」を参照。

なお、この適合コードは「LCK04-J. 基になるコレクションにアクセス可能な場合にはコレクションビューを使って同期しない」には違反していない。コレクションビュー(synchronizedList)に対して同期しているが、基となるコレクションへのアクセスが不可能であるため、コレクションはいかなるコードからも変更されてない。

なお、この適合コードは、実際にはCollections.synchronizedList()が提供する同期メカニズムを使用していない。他の部分でも全く使わないのであれば、synchronizedList でラップする必要はないだろう。

違反コード (synchronizedMap()メソッド)

以下の違反コードでは、スレッドセーフではない keyedCounter クラスを定義している。HashMapsynchronizedMap() の中にラップされているが、increment() の処理全体はアトミックではない。[Lee 2009]。

final class KeyedCounter {
  private final Map<String, Integer> map =
      Collections.synchronizedMap(new HashMap<String, Integer>());

  public void increment(String key) {
    Integer old = map.get(key);
    int oldValue = (old == null) ? 0 : old.intValue();
    if (oldValue == Integer.MAX_VALUE) {
      throw new ArithmeticException("Out of range");
    }
    map.put( key, oldValue + 1);
  }

  public Integer getCount(String key) {
    return map.get(key);
  }
}
適合コード (同期)

以下の適合コードでは、アトミック性を確保するため、private宣言されたロックオブジェクトを使用することでincrement()メソッドとgetCount()メソッドを同期している。

final class KeyedCounter {
  private final Map<String, Integer> map =
      new HashMap<String, Integer>();
  private final Object lock = new Object();

  public void increment(String key) {
    synchronized (lock) {
      Integer old = map.get(key);
      int oldValue = (old == null) ? 0 : old.intValue();
      if (oldValue == Integer.MAX_VALUE) {
        throw new ArithmeticException("Out of range");
      }
      map.put(key, oldValue + 1);
    }
  }

  public Integer getCount(String key) {
    synchronized (lock) {
      return map.get(key);
    }
  }
}

この適合コードではCollections.synchronizedMap()を使用していない。synchronizedしていないmapに対してロックを行うことで十分スレッドセーフになるからである。synchronizedMapオブジェクトを同期する方法については「LCK04-J. 基になるコレクションにアクセス可能な場合にはコレクションビューを使って同期しない」が詳しい。

適合コード (ConcurrentHashMap) クラス

前述の適合コードはマルチスレッド環境で安全に使用することができるが、スケーラビリティが悪く、過度な同期が行われる結果、性能が低下する。

以下の適合コードで使用するConcurrentHashMapクラスではアトミックな操作を行うためのユーティリティメソッドが実装されており、スケーラビリティが求められるアルゴリズムでの使用に適している。

この適合コードでは依然として同期が必要であることに注意。同期を行わないと、オーバーフローを防止するための検査と値のインクリメントがアトミックに行われず、incrementを呼び出す2つのスレッドがオーバーフローを引き起こす可能性がある。同期ブロックはより小さくなり、新規の値の読出しや追加は行われないため、前述の適合コードほど性能上の影響はない。

final class KeyedCounter {
  private final ConcurrentMap<String, AtomicInteger> map =
      new ConcurrentHashMap<String, AtomicInteger>();

  public void increment(String key) {
    AtomicInteger value = new AtomicInteger();
    AtomicInteger old = map.putIfAbsent(key, value);

    if (old != null) {
      value = old;
    }

    synchronized (lock) {
      if (value.get() == Integer.MAX_VALUE) {
        throw new ArithmeticException("Out of range");
      }
      value.incrementAndGet(); // 値をアトミックにインクリメントする
    }
  }

  public Integer getCount(String key) {
    AtomicInteger value = map.get(key);
    return (value == null) ? null : value.get();
  }

  // 他のアクセッサ ...
}

Goetzの「Java並行処理プログラミング」5.2.1節「ConcurrentHashMap」には以下のように記述されている。[Goetz 2006]

ConcurrentHashMapとその他の並行処理コレクションクラスは、ConcurrentModificationExceptionを投げないイテレータによりイテレーション中のロックを不要とするなど同期コレクションクラスをさらに改善している。ConcurrentHashMapが返すイテレータは、即時断念しない弱い一貫性を保つ。弱い一貫性を保つイテレータは、並行する更新を許し、イテレータが構築された時点のコレクション要素を対象として走査する。さらに、保証はされないが、イテレータが構築された後に行われた更新をも反映しうる。

なお、性能上の理由から、ConcurrentHashMap.size()ConcurrentHashMap.isEmpty()は、近似値を返すことが許されている。正確な結果が求められるコードではこれらのメソッドの返り値に依存すべきではない。

リスク評価

マルチスレッドアプリケーションにおいて、単一のアトミック動作として実行されるべき複数操作のアトミック性を確保できない場合、競合状態の発生につながる恐れがある。

ルール 深刻度 可能性 修正コスト 優先度 レベル
VNA03-J P4 L3
関連ガイドライン
MITRE CWE CWE-362. Concurrent execution using shared resource with improper synchronization ("race condition")
  CWE-366. Race condition within a thread
  CWE-662. Improper synchronization
参考文献
[API 2006]  
[Goetz 2006] Section 4.4.1, Client-side Locking
  Section 5.2.1, ConcurrentHashMap
[JavaThreads 2004] Section 8.2, Synchronization and Collection Classes
[Lee 2009] Map & Compound Operation
翻訳元

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

VNA03-J. Do not assume that a group of calls to independently atomic methods is atomic (revision 185)

Top へ

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