7132889: (se) AbstractSelectableChannel.register and configureBlocking not safe from asynchronous close

Reviewed-by: alanb

jdk/src/share/classes/java/nio/channels/spi/AbstractSelectableChannel.java

@@ -90,7 +90,7 @@ public abstract class AbstractSelectableChannel
     // -- Utility methods for the key set --
 
     private void addKey(SelectionKey k) {
-        synchronized (keyLock) {
+        assert Thread.holdsLock(keyLock);
         int i = 0;
         if ((keys != null) && (keyCount < keys.length)) {
             // Find empty element of key array
@@ -111,7 +111,6 @@ public abstract class AbstractSelectableChannel
         keys[i] = k;
         keyCount++;
     }
-    }
 
     private SelectionKey findKey(Selector sel) {
         synchronized (keyLock) {
@@ -190,11 +189,11 @@ public abstract class AbstractSelectableChannel
                                        Object att)
         throws ClosedChannelException
     {
+        synchronized (regLock) {
             if (!isOpen())
                 throw new ClosedChannelException();
             if ((ops & ~validOps()) != 0)
                 throw new IllegalArgumentException();
-        synchronized (regLock) {
             if (blocking)
                 throw new IllegalBlockingModeException();
             SelectionKey k = findKey(sel);
@@ -204,9 +203,13 @@ public abstract class AbstractSelectableChannel
             }
             if (k == null) {
                 // New registration
+                synchronized (keyLock) {
+                    if (!isOpen())
+                        throw new ClosedChannelException();
                     k = ((AbstractSelector)sel).register(this, ops, att);
                     addKey(k);
                 }
+            }
             return k;
         }
     }
@@ -275,9 +278,9 @@ public abstract class AbstractSelectableChannel
     public final SelectableChannel configureBlocking(boolean block)
         throws IOException
     {
+        synchronized (regLock) {
             if (!isOpen())
                 throw new ClosedChannelException();
-        synchronized (regLock) {
             if (blocking == block)
                 return this;
             if (block && haveValidKeys())

jdk/test/java/nio/channels/SelectionKey/RacyRegister.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 7132889
+ * @summary Test that register does not return a valid SelectionKey when
+ *    invoked at around the time that the channel is closed
+ */
+
+import java.nio.channels.*;
+import java.util.concurrent.*;
+import java.util.Random;
+import java.io.IOException;
+
+public class RacyRegister {
+
+    public static void main(String[] args) throws Exception {
+        ExecutorService pool = Executors.newFixedThreadPool(1);
+        try (Selector sel = Selector.open()) {
+            int count = 100;
+            while (count-- > 0) {
+                final SocketChannel sc = SocketChannel.open();
+                sc.configureBlocking(false);
+
+                // close channel asynchronously
+                Future<Void> result = pool.submit(new Callable<Void>() {
+                    public Void call() throws IOException {
+                        sc.close();
+                        return null;
+                    }
+                });
+
+                // attempt to register channel with Selector
+                SelectionKey key = null;
+                try {
+                    key = sc.register(sel, SelectionKey.OP_READ);
+                } catch (ClosedChannelException ignore) {
+                }
+
+                // ensure close is done
+                result.get();
+
+                // if we have a key then it should be invalid
+                if (key != null && key.isValid())
+                    throw new RuntimeException("Key is valid");
+            }
+        } finally {
+            pool.shutdown();
+        }
+    }
+}