stateLock and closeLock need to be ReentrantLock niosocketimpl-branch
authoralanb
Sat, 13 Apr 2019 07:22:55 +0100
branchniosocketimpl-branch
changeset 57321 eef9324f94cc
parent 57313 14b02c7b27b8
child 57322 4744fdcf458c
stateLock and closeLock need to be ReentrantLock
src/java.base/share/classes/java/net/ServerSocket.java
src/java.base/share/classes/java/net/Socket.java
src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java
--- a/src/java.base/share/classes/java/net/ServerSocket.java	Tue Apr 09 12:02:23 2019 +0100
+++ b/src/java.base/share/classes/java/net/ServerSocket.java	Sat Apr 13 07:22:55 2019 +0100
@@ -34,6 +34,7 @@
 import java.security.PrivilegedExceptionAction;
 import java.util.Set;
 import java.util.Collections;
+import java.util.concurrent.locks.ReentrantLock;
 
 import jdk.internal.access.JavaNetSocketAccess;
 import jdk.internal.access.SharedSecrets;
@@ -64,7 +65,7 @@
     private boolean created = false;
     private boolean bound = false;
     private boolean closed = false;
-    private Object closeLock = new Object();
+    private final ReentrantLock closeLock = new ReentrantLock();
 
     /**
      * The implementation of this Socket.
@@ -686,12 +687,19 @@
      * @spec JSR-51
      */
     public void close() throws IOException {
-        synchronized(closeLock) {
-            if (isClosed())
-                return;
-            if (created)
-                impl.close();
-            closed = true;
+        closeLock.lock();
+        try {
+            if (!closed) {
+                try {
+                    SocketImpl impl = this.impl;
+                    if (impl != null)
+                        impl.close();
+                } finally {
+                    closed = true;
+                }
+            }
+        } finally {
+            closeLock.unlock();
         }
     }
 
@@ -733,8 +741,11 @@
      * @since 1.4
      */
     public boolean isClosed() {
-        synchronized(closeLock) {
+        closeLock.lock();
+        try {
             return closed;
+        } finally {
+            closeLock.unlock();
         }
     }
 
--- a/src/java.base/share/classes/java/net/Socket.java	Tue Apr 09 12:02:23 2019 +0100
+++ b/src/java.base/share/classes/java/net/Socket.java	Sat Apr 13 07:22:55 2019 +0100
@@ -35,6 +35,7 @@
 import java.security.PrivilegedAction;
 import java.util.Set;
 import java.util.Collections;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * This class implements client sockets (also called just
@@ -62,7 +63,7 @@
     private boolean bound = false;
     private boolean connected = false;
     private boolean closed = false;
-    private Object closeLock = new Object();
+    private final ReentrantLock closeLock = new ReentrantLock();
     private boolean shutIn = false;
     private boolean shutOut = false;
 
@@ -1573,13 +1574,20 @@
      * @spec JSR-51
      * @see #isClosed
      */
-    public synchronized void close() throws IOException {
-        synchronized(closeLock) {
-            if (isClosed())
-                return;
-            if (created)
-                impl.close();
-            closed = true;
+    public void close() throws IOException {
+        closeLock.lock();
+        try {
+            if (!closed) {
+                try {
+                    SocketImpl impl = this.impl;
+                    if (impl != null)
+                        impl.close();
+                } finally {
+                    closed = true;
+                }
+            }
+        } finally {
+            closeLock.unlock();
         }
     }
 
@@ -1700,8 +1708,11 @@
      * @see #close
      */
     public boolean isClosed() {
-        synchronized(closeLock) {
+        closeLock.lock();
+        try {
             return closed;
+        } finally {
+            closeLock.unlock();
         }
     }
 
--- a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java	Tue Apr 09 12:02:23 2019 +0100
+++ b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java	Sat Apr 13 07:22:55 2019 +0100
@@ -48,6 +48,7 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.ReentrantLock;
 
 import jdk.internal.ref.CleanerFactory;
@@ -90,7 +91,8 @@
     private final ReentrantLock writeLock = new ReentrantLock();
 
     // The stateLock for read/changing state
-    private final Object stateLock = new Object();
+    private final ReentrantLock stateLock = new ReentrantLock();
+    private final Condition stateCondition = stateLock.newCondition();
     private static final int ST_NEW = 0;
     private static final int ST_UNCONNECTED = 1;
     private static final int ST_CONNECTING = 2;
@@ -206,10 +208,13 @@
      * @throws SocketException if the socket is closed or not connected
      */
     private FileDescriptor beginRead() throws SocketException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpenAndConnected();
             readerThread = NativeThread.current();
             return fd;
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -218,13 +223,16 @@
      * @throws SocketException is the socket is closed
      */
     private void endRead(boolean completed) throws SocketException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             readerThread = 0;
             int state = this.state;
             if (state == ST_CLOSING)
-                stateLock.notifyAll();
+                stateCondition.signalAll();
             if (!completed && state >= ST_CLOSING)
                 throw new SocketException("Socket closed");
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -349,10 +357,13 @@
      * @throws SocketException if the socket is closed or not connected
      */
     private FileDescriptor beginWrite() throws SocketException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpenAndConnected();
             writerThread = NativeThread.current();
             return fd;
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -361,13 +372,16 @@
      * @throws SocketException is the socket is closed
      */
     private void endWrite(boolean completed) throws SocketException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             writerThread = 0;
             int state = this.state;
             if (state == ST_CLOSING)
-                stateLock.notifyAll();
+                stateCondition.signalAll();
             if (!completed && state >= ST_CLOSING)
                 throw new SocketException("Socket closed");
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -439,7 +453,8 @@
      */
     @Override
     protected void create(boolean stream) throws IOException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             if (state != ST_NEW)
                 throw new IOException("Already created");
             if (!stream)
@@ -461,6 +476,8 @@
             this.stream = stream;
             this.closer = FileDescriptorCloser.create(this);
             this.state = ST_UNCONNECTED;
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -471,7 +488,8 @@
     private FileDescriptor beginConnect(InetAddress address, int port)
         throws IOException
     {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             int state = this.state;
             if (state != ST_UNCONNECTED) {
                 if (state == ST_NEW)
@@ -497,6 +515,8 @@
 
             readerThread = NativeThread.current();
             return fd;
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -505,17 +525,20 @@
      * @throws SocketException is the socket is closed
      */
     private void endConnect(FileDescriptor fd, boolean completed) throws IOException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             readerThread = 0;
             int state = this.state;
             if (state == ST_CLOSING)
-                stateLock.notifyAll();
+                stateCondition.signalAll();
             if (completed && state == ST_CONNECTING) {
                 this.state = ST_CONNECTED;
                 localport = Net.localAddress(fd).getPort();
             } else if (!completed && state >= ST_CLOSING) {
                 throw new SocketException("Socket closed");
             }
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -615,7 +638,8 @@
 
     @Override
     protected void bind(InetAddress host, int port) throws IOException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpen();
             if (localport != 0)
                 throw new SocketException("Already bound");
@@ -626,16 +650,21 @@
             // then the actual local address will be ::0 when IPv6 is enabled.
             address = host;
             localport = Net.localAddress(fd).getPort();
+        } finally {
+            stateLock.unlock();
         }
     }
 
     @Override
     protected void listen(int backlog) throws IOException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpen();
             if (localport == 0)
                 throw new SocketException("Not bound");
             Net.listen(fd, backlog < 1 ? 50 : backlog);
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -644,7 +673,8 @@
      * @throws SocketException if the socket is closed
      */
     private FileDescriptor beginAccept() throws SocketException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpen();
             if (!stream)
                 throw new SocketException("Not a stream socket");
@@ -652,6 +682,8 @@
                 throw new SocketException("Not bound");
             readerThread = NativeThread.current();
             return fd;
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -660,13 +692,16 @@
      * @throws SocketException is the socket is closed
      */
     private void endAccept(boolean completed) throws SocketException {
-        synchronized (stateLock) {
+        stateLock.lock(); 
+        try {
             int state = this.state;
             readerThread = 0;
             if (state == ST_CLOSING)
-                stateLock.notifyAll();
+                stateCondition.signalAll();
             if (!completed && state >= ST_CLOSING)
                 throw new SocketException("Socket closed");
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -764,7 +799,8 @@
         }
 
         // set the fields
-        synchronized (nsi.stateLock) {
+        nsi.stateLock.lock();
+        try {
             nsi.fd = newfd;
             nsi.stream = true;
             nsi.closer = FileDescriptorCloser.create(nsi);
@@ -772,6 +808,8 @@
             nsi.address = isaa[0].getAddress();
             nsi.port = isaa[0].getPort();
             nsi.state = ST_CONNECTED;
+        } finally {
+            nsi.stateLock.unlock();
         }
     }
 
@@ -820,13 +858,16 @@
 
     @Override
     protected int available() throws IOException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpenAndConnected();
             if (isInputClosed) {
                 return 0;
             } else {
                 return Net.available(fd);
             }
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -838,7 +879,8 @@
     protected void close() throws IOException {
         boolean interrupted = false;
 
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             int state = this.state;
             if (state >= ST_CLOSING)
                 return;
@@ -872,7 +914,7 @@
                 // wait for blocking I/O operations to end
                 while (readerThread != 0 || writerThread != 0) {
                     try {
-                        stateLock.wait();
+                        stateCondition.await();
                     } catch (InterruptedException e) {
                         interrupted = true;
                     }
@@ -885,6 +927,8 @@
             } finally {
                 this.state = ST_CLOSED;
             }
+        } finally {
+            stateLock.unlock();
         }
 
         // restore interrupt status
@@ -931,7 +975,8 @@
     protected <T> void setOption(SocketOption<T> opt, T value) throws IOException {
         if (!supportedOptions().contains(opt))
             throw new UnsupportedOperationException("'" + opt + "' not supported");
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpen();
             if (opt == StandardSocketOptions.IP_TOS) {
                 // maps to IP_TOS or IPV6_TCLASS
@@ -949,6 +994,8 @@
                 // option does not need special handling
                 Net.setSocketOption(fd, opt, value);
             }
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -956,7 +1003,8 @@
     protected <T> T getOption(SocketOption<T> opt) throws IOException {
         if (!supportedOptions().contains(opt))
             throw new UnsupportedOperationException("'" + opt + "' not supported");
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpen();
             if (opt == StandardSocketOptions.IP_TOS) {
                 return (T) Integer.valueOf(trafficClass);
@@ -970,6 +1018,8 @@
                 // option does not need special handling
                 return (T) Net.getSocketOption(fd, opt);
             }
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -987,7 +1037,8 @@
 
     @Override
     public void setOption(int opt, Object value) throws SocketException {
-        synchronized (stateLock) {
+        stateLock.lock(); 
+        try {
             ensureOpen();
             try {
                 switch (opt) {
@@ -1068,12 +1119,15 @@
             } catch (IllegalArgumentException | IOException e) {
                 throw new SocketException(e.getMessage());
             }
+        } finally {
+            stateLock.unlock();
         }
     }
 
     @Override
     public Object getOption(int opt) throws SocketException {
-        synchronized (stateLock) {
+        stateLock.lock(); 
+        try {
             ensureOpen();
             try {
                 switch (opt) {
@@ -1120,28 +1174,36 @@
             } catch (IllegalArgumentException | IOException e) {
                 throw new SocketException(e.getMessage());
             }
+        } finally {
+            stateLock.unlock();
         }
     }
 
     @Override
     protected void shutdownInput() throws IOException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpenAndConnected();
             if (!isInputClosed) {
                 Net.shutdown(fd, Net.SHUT_RD);
                 isInputClosed = true;
             }
+        } finally {
+            stateLock.unlock();
         }
     }
 
     @Override
     protected void shutdownOutput() throws IOException {
-        synchronized (stateLock) {
+        stateLock.lock();
+        try {
             ensureOpenAndConnected();
             if (!isOutputClosed) {
                 Net.shutdown(fd, Net.SHUT_WR);
                 isOutputClosed = true;
             }
+        } finally {
+            stateLock.unlock();
         }
     }
 
@@ -1199,7 +1261,7 @@
         }
 
         static FileDescriptorCloser create(NioSocketImpl impl) {
-            assert Thread.holdsLock(impl.stateLock);
+            assert impl.stateLock.isHeldByCurrentThread();
             var closer = new FileDescriptorCloser(impl.fd, impl.stream);
             CleanerFactory.cleaner().register(impl, closer);
             return closer;