JDK-6436220 : (se) SelectionKey.attach is not atomic
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.nio
  • Affected Version: 6
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2006-06-08
  • Updated: 2011-05-18
  • Resolved: 2011-05-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7
7 b06Fixed
Description
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);
                }
            }
        };
    }
}

Comments
EVALUATION As tackline has identified, the attach method is not atomic and two (or more) threads attempting to update the attachment at the same time could end up being returned the same previous attachment. I'm not aware of any applications where this could occur in practice but we should fix it. The proposed changes are good.
11-06-2006