JDK-4505651 : Synchronized collection wrappers not correctly synchronized
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util
  • Affected Version: 1.4.0
  • Priority: P2
  • Status: Closed
  • Resolution: Not an Issue
  • OS: generic
  • CPU: generic
  • Submitted: 2001-09-21
  • Updated: 2001-09-28
  • Resolved: 2001-09-28
Description

Name: nt126004			Date: 09/21/2001


java version "1.4.0-beta2"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.0-beta2-b77)
Java HotSpot(TM) Client VM (build 1.4.0-beta2-b77, mixed mode)


The synchronized collection wrappers don't synchronize properly and can cause
non-deterministic behaviour when batch operations (e.g. addAll) are invoked.

The wrappers generally synchronize on the mutex before delegating the operation
to the underlying collection.  The underlying collection then typically
iterates over the argument collection, performing some action for each
element.  If the argument collection is a synchronized wrapper, all iteration
on it is supposed to be guarded by a synchronization block (see
Collections.synchronizedCollection); if this is not done, the documentation
warns of possible non-deterministic behaviour.  However, the iteration over the
argument collection is never done under a synchronization lock, and is thus
vulnerable.

The following code will throw a ConcurrentModificationException, even though
both collections are only being accessed through the synchronized wrappers.

import java.util.*;
public class SyncTest {
  public static void main(String[] args) {
    final Collection c1 = Collections.synchronizedCollection(new ArrayList());
    final Collection c2 = Collections.synchronizedCollection(new ArrayList());
    c1.add("a");  c1.add("b");
    new Thread(new Runnable() {
      public void run() {
        while(true) {
          c1.add("c");
          c1.remove("c");
        }
      }
    }).start();
    while(true) {
      c2.addAll(c1);
      c2.removeAll(c1);
    }
  }
}
(Review ID: 132344) 
======================================================================

Comments
WORK AROUND Name: nt126004 Date: 09/21/2001 Customer Workaround: Manually synchronize on the argument collection while calling the batch operation, if ones guesses that it may iterate over its argument. This is not satisfactory since one now has to know whether any given collection instance is supposed to be synchronized or not. This knowledge is not always available at the point a collection is operated on. ======================================================================
11-06-2004

EVALUATION The reporter's program is at fault. The problem is that the addAll call (not the reomveAll call!) interates over its argument collection but cannot (and should not attempt to) synchronize on it. The documentation for the synchronization wrappers (and for all synchronized collections) is quite explicit about the fact that one must synchronize manually during iteration. To fix the program, replace this line: c2.addAll(c1); with these: synchronized(c1) { c2.addAll(c1); } Note that the line c2.removeAll(c1); Locks c2 before c1, whereas the above lines lock the two collecitons in the opposite order. This would be deadlock prone if the two activities occurred in different threads, but they do not. If they did, the deadlock could be prevented by surrounding the call to removeAll in a synchronized(c1) { } block. ###@###.### 2001-09-28
28-09-2001