# HG changeset patch # User alanb # Date 1555136575 -3600 # Node ID eef9324f94cc9f93642514ee89f453904581bd09 # Parent 14b02c7b27b882f88489e5fbc94627bc86a0986d stateLock and closeLock need to be ReentrantLock diff -r 14b02c7b27b8 -r eef9324f94cc src/java.base/share/classes/java/net/ServerSocket.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(); } } diff -r 14b02c7b27b8 -r eef9324f94cc src/java.base/share/classes/java/net/Socket.java --- 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(); } } diff -r 14b02c7b27b8 -r eef9324f94cc src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java --- 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 void setOption(SocketOption 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 getOption(SocketOption 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;