Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Edited by sciSpider Java v3.0

...

Code Block
bgColor#FFCCCC
class RaceCollection implements Runnable {
  private List<InetAddress>List&lt;InetAddress&gt; ips = Collections.synchronizedList(new ArrayList<InetAddress>ArrayList&lt;InetAddress&gt;());
  
  public void addIPAddress(InetAddress ia) {
    synchronized(ips) {
      ips.add(ia);
    }
  }

  public void removeIPAddress(InetAddress ia) {
    synchronized(ips) {
      ips.remove(ia);
    }
  }

  public void nonAtomic() throws InterruptedException {
    InetAddress[] ia;   

    synchronized(ips) {
      ia = (InetAddress[]) ips.toArray(new InetAddress[0]);     
    }
         
    // This statement should be in the synchronized block above 
    System.out.println("&quot;Number of IPs: "&quot; + ia.length); 
  }
  
  public void run() {
    try {
      addIPAddress(InetAddress.getLocalHost());
      nonAtomic();
    } catch (UnknownHostException e) { /* Forward to handler */ }
      catch (InterruptedException e) { /* Forward to handler */ }		
  }
  
  public static void main(String[] args) {
    RaceCollection rc1 = new RaceCollection();
    for(int i = 0; i <&lt; 2; i++) {
      new Thread(rc1).start();
    }	  	  
  }
}

...

Code Block
bgColor#ccccff
synchronized(ips) {
  ia = (InetAddress[]) ips.toArray(new InetAddress[0]);           
  System.out.println("&quot;Number of IPs: "&quot; + ia.length); 
}

Note that this advice applies to all Collection classes including the thread-safe Hashtable class. Enumerations of the objects of a Collection and iterators also require explicit synchronization on the Collection object or any single lock object.

...

Code Block
bgColor#ccccff
class CompositeCollection implements Runnable {
  private List<InetAddress>List&lt;InetAddress&gt; ips = Collections.synchronizedList(new ArrayList<InetAddress>ArrayList&lt;InetAddress&gt;());
  public CompositeCollection(List<InetAddress>List&lt;InetAddress&gt; list) {
    this.ips = list;
  }
  
  public synchronized void addIPAddress(InetAddress ia) {
    ips.add(ia);
  }

  // Other methods

  public synchronized void atomic() throws InterruptedException {
    InetAddress[] ia;   
    ia = (InetAddress[]) ips.toArray(new InetAddress[0]);     
    System.out.println("&quot;Number of IPs: "&quot; + ia.length); 
  }
}

Wiki Markup
Yet another method is to extend the base class and synchronize on the method that is desired to be atomic, however, it is not recommended because it goes against the spirit of limiting class extension ([OBJ33-J. Limit the extensibility of non-final classes and methods to only trusted subclasses]). Moreover, Goetz et al. \[[Goetz 06|AA. Java References#Goetz 06]\] cite other reasons:

...

Code Block
bgColor#FFCCCC
public class KeyedCounter {
  private Map<StringMap&lt;String, Integer>Integer&gt; map =
    Collections.synchronizedMap(new HashMap<StringHashMap&lt;String, Integer>Integer&gt;());

  public void increment(String key) {
    Integer old = map.get(key);
    int value = (old == null) ? 1 : old.intValue() + 1;
    map.put(key, value);
  }

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

...

Code Block
bgColor#ccccff
public class KeyedCounter {
  private Map<String,Integer>Map&lt;String,Integer&gt; map = new HashMap<String,Integer>()HashMap&lt;String,Integer&gt;();
 
  public synchronized void increment(String key) {
    Integer old = map.get(key);
    int value = (old == null) ? 1 : old.intValue()+1;
    map.put(key, value);
  }

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

...

ConcurrentHashMap, along with the other concurrent collections, further improve on the synchronized collection classes by providing iterators that do not throw ConcurrentModificationException, thus as a result eliminating the need to lock the collection during iteration. The iterators returned by ConcurrentHashMap are weakly consistent instead of fail-fast. A weakly consistent iterator can tolerate concurrent modification, traverses elements as they existed when the iterator was constructed, and may (but is not guaranteed to) reflect modifications to the collection after the construction of the iterator.

Code Block
bgColor#ccccff
public class KeyedCounter {
  private final ConcurrentMap<StringConcurrentMap&lt;String, AtomicInteger>AtomicInteger&gt; map =
    new ConcurrentHashMap<StringConcurrentHashMap&lt;String, AtomicInteger>AtomicInteger&gt;();

  public void increment(String key) {
    AtomicInteger value = new AtomicInteger(0);
    AtomicInteger old = map.putIfAbsent(key, value);
   
    if (old != null) { 
      value = old; 
    }

    value.incrementAndGet(); // Increment the value atomically
  }

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

...

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

Wiki Markup
\[[API 06|AA. Java References#API 06]\] Class Vector, Class WeakReference
\[[JavaThreads 04|AA. Java References#JavaThreads 04]\] 8.2 "&quot;Synchronization and Collection Classes"&quot;
\[[Goetz 06|AA. Java References#Goetz 06]\] 4.4.1. Client-side Locking, 4.4.2. Composition and 5.2.1. ConcurrentHashMap
\[[Lee 09|AA. Java References#Lee 09]\] "&quot;Map &amp; Compound Operation"&quot;

...

CON37-J. Never apply a lock to methods making network calls      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;11. Concurrency (CON)      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;CON39-J. Ensure atomicity of 64-bit operations