A DESCRIPTION OF THE FIX : java.nio.channels.SelectionKey.attach is not atomic. SelectionKey.attach swaps the value of a volatile variable. However, it does not perform the operation atomically. Therefore, some values may be extracted multiple times and others lost. AtomicReferenceFieldUpdater.getAndSet provides an excellent way to ensure atomic operation. Note, on a machine with a single processor, -Xint is useful to quite a reliable failure in the test code. Diffs against 1.6.0-beta2-b85 --- /home/tackline/mustang/current/j2se/src/share/classes/java/nio/channels/SelectionKey.java 2006-05-26 12:02:02.000000000 +0100 +++ java/nio/channels/SelectionKey.java 2006-05-29 20:01:03.000000000 +0100 @@ -8,7 +8,7 @@ package java.nio.channels; import java.io.IOException; - +import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; /** * A token representing the registration of a {@link SelectableChannel} with a @@ -346,6 +346,14 @@ // -- Attachments -- private volatile Object attachment = null; + + /** + * Updates {@link #attachment} variable atomically. + */ + private static final AtomicReferenceFieldUpdater<SelectionKey,Object> + attachmentUpdater = AtomicReferenceFieldUpdater.newUpdater( + SelectionKey.class, Object.class, "attachment" + ); /** * Attaches the given object to this key. @@ -362,9 +370,7 @@ * otherwise <tt>null</tt> */ public final Object attach(Object ob) { - Object a = attachment; - attachment = ob; - return a; + return attachmentUpdater.getAndSet(this, ob); } /** JUnit TESTCASE : import java.nio.channels.*; import java.util.concurrent.atomic.AtomicBoolean; public class AttachmentTest extends junit.framework.TestCase { public static void main(String[] args) { junit.textui.TestRunner.run(AttachmentTest.class); } /** * More likely to find the problem with -Xint * (certainly on a single processor machine). */ public void testAttach() throws Exception { Selector selector = Selector.open(); Pipe pipe = Pipe.open(); SelectableChannel channel = pipe.sink().configureBlocking(false); final SelectionKey key = channel.register(selector, 0); key.attach(new AtomicBoolean()); new Thread() { private final AtomicBoolean error = new AtomicBoolean(); private int failCount; public void run() { AtomicBoolean att = new AtomicBoolean(); for (int ct=0; ct<1000*1000 && !error.get(); ++ct) { // att.get() should be false. att = (AtomicBoolean)key.attach(att); // We should have exclusive ownership of att. if ( att.compareAndSet(false, true) && att.compareAndSet(true, false) ) { // Good stuff. continue; } else { // Not good. failCount = ct; error.set(true); break; } } } { start(); run(); if (error.get()) { fail("attach not atomic, on count: "+failCount); } } }; } }
|