Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: added CSs

...

Code Block
bgColor#FFCCCC
class RaceCollection implements Runnable {
  private List<InetAddress> ips = Collections.synchronizedList(new ArrayList<InetAddress>());
  
  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]);     
    }
         
    System.out.println("Number of IPs: " + ia.length); 
  }
  
  public void run() {
    try {
      addIPAddress(InetAddress.getLocalHost());
      nonAtomic();
    } catch (UnknownHostException e) { }
      catch (InterruptedException e) { }		
  }
  
  public static void main(String[] args) {
    RaceCollection rc1 = new RaceCollection();
    for(int i=0;i<2;i++)
      new Thread(rc1).start();	  	  
  }
}

Compliant Solution

Wiki Markup
To eliminate the race condition, ensure atomicity. This can be achieved by including all statements that use the array list within the synchronized block. This technique is also called client-side locking. \[[Goetz 06|AA. Java References#Goetz 06]\]

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

...

Wiki Markup
Although expensive, {{CopyOnWriteArrayList}} and {{CopyOnWriteArraySet}} classes are sometimes used to create copies of the core {{Collection}} so that iterators do not fail with a runtime exception when some data in the {{Collection}} is modified. These however, suffer from the {{toArray}} dilemma (operating on stale data) described earlier in this rule. Therefore their use is limited to boosting performance in code where the writes are fewer (or non-existent) as compared to the reads  \[[JavaThreads 04|AA. Java References#JavaThreads 04]\]. In all other cases they must be avoided.    

Compliant Solution

Wiki Markup
Composition offers more benefits than the previous solution at the cost of a slight performance penalty (refer to [OBJ01-J. Understand how a superclass can affect a subclass] for details on how to implement composition). This allows the {{CompositeCollection}} class to use its own intrinsic lock in a way that is completely independent of the lock of the underlying list class. This provides consistent locking even when the underlying list is not thread-safe or when it changes its locking policy. \[[Goetz 06|AA. Java References#Goetz 06]\]

Code Block
bgColor#ccccff

class CompositeCollection implements Runnable {
  private List<InetAddress> ips = Collections.synchronizedList(new ArrayList<InetAddress>());
  public CompositeCollection(List<InetAddress> 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("Number of IPs: " + 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 classes and methods]). Moreover, Goetz et al. \[[Goetz 06|AA. Java References#Goetz 06]\] cite other reasons:

Extension is more fragile than adding code directly to a class, because the implementation of the synchronization policy is now distributed over multiple, separately maintained source files. If the underlying class were to change its synchronization policy by choosing a different lock to guard its state variables, the subclass would subtly and silently break, because it no longer used the right lock to control concurrent access to the base class's state.

Risk Assessment

Non-atomic code can induce race conditions and affect program correctness.

...

Wiki Markup
\[[API 06|AA. Java References#API 06]\] Class Vector, Class WeakReference
\[[JavaThreads 04|AA. Java References#JavaThreads 04]\] 8.2 "Synchronization and Collection Classes"
\[[Goetz 06|AA. Java References#Goetz 06]\] 4.4.1. Client-side Locking and 4.4.2. Composition

...

MSC00-J. Eliminate class initialization cycles      49. Miscellaneous (MSC)      MSC02-J. Be aware of the JVM Tool Interface