8199120: (so) SocketChannelImpl read/write don't need stateLock when channel is configured non-blocking
authoralanb
Wed, 07 Mar 2018 07:20:38 +0000
changeset 49143 2d5cc05d877e
parent 49142 4affaea00c05
child 49144 71bc133f25ea
child 56255 39e28481492d
8199120: (so) SocketChannelImpl read/write don't need stateLock when channel is configured non-blocking Reviewed-by: bpb
src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java
--- a/src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java	Wed Mar 07 07:15:24 2018 +0000
+++ b/src/java.base/share/classes/sun/nio/ch/SocketChannelImpl.java	Wed Mar 07 07:20:38 2018 +0000
@@ -148,12 +148,37 @@
         }
     }
 
-    // @throws ClosedChannelException if channel is closed
+    /**
+     * Checks that the channel is open.
+     *
+     * @throws ClosedChannelException if channel is closed (or closing)
+     */
     private void ensureOpen() throws ClosedChannelException {
         if (!isOpen())
             throw new ClosedChannelException();
     }
 
+    /**
+     * Checks that the channel is open and connected.
+     *
+     * @apiNote This method uses the "state" field to check if the channel is
+     * open. It should never be used in conjuncion with isOpen or ensureOpen
+     * as these methods check AbstractInterruptibleChannel's closed field - that
+     * field is set before implCloseSelectableChannel is called and so before
+     * the state is changed.
+     *
+     * @throws ClosedChannelException if channel is closed (or closing)
+     * @throws NotYetConnectedException if open and not connected
+     */
+    private void ensureOpenAndConnected() throws ClosedChannelException {
+        int state = this.state;
+        if (state < ST_CONNECTED) {
+            throw new NotYetConnectedException();
+        } else if (state > ST_CONNECTED) {
+            throw new ClosedChannelException();
+        }
+    }
+
     @Override
     public Socket socket() {
         synchronized (stateLock) {
@@ -275,13 +300,14 @@
         if (blocking) {
             // set hook for Thread.interrupt
             begin();
-        }
-        synchronized (stateLock) {
-            ensureOpen();
-            if (state != ST_CONNECTED)
-                throw new NotYetConnectedException();
-            if (blocking)
+
+            synchronized (stateLock) {
+                ensureOpenAndConnected();
+                // record thread so it can be signalled if needed
                 readerThread = NativeThread.current();
+            }
+        } else {
+            ensureOpenAndConnected();
         }
     }
 
@@ -385,15 +411,16 @@
         if (blocking) {
             // set hook for Thread.interrupt
             begin();
-        }
-        synchronized (stateLock) {
-            ensureOpen();
-            if (isOutputClosed)
-                throw new ClosedChannelException();
-            if (state != ST_CONNECTED)
-                throw new NotYetConnectedException();
-            if (blocking)
+
+            synchronized (stateLock) {
+                ensureOpenAndConnected();
+                if (isOutputClosed)
+                    throw new ClosedChannelException();
+                // record thread so it can be signalled if needed
                 writerThread = NativeThread.current();
+            }
+        } else {
+            ensureOpenAndConnected();
         }
     }
 
@@ -612,8 +639,10 @@
                 NetHooks.beforeTcpConnect(fd, isa.getAddress(), isa.getPort());
             remoteAddress = isa;
 
-            if (blocking)
+            if (blocking) {
+                // record thread so it can be signalled if needed
                 readerThread = NativeThread.current();
+            }
         }
     }
 
@@ -695,8 +724,10 @@
             ensureOpen();
             if (state != ST_CONNECTIONPENDING)
                 throw new NoConnectionPendingException();
-            if (blocking)
+            if (blocking) {
+                // record thread so it can be signalled if needed
                 readerThread = NativeThread.current();
+            }
         }
     }