# HG changeset patch # User alanb # Date 1551171196 0 # Node ID 4503441bec2e55e2033f341e846590c37183f625 # Parent a67ea4f53e5666fa96399e36defd435d14eee41d More NioSocketImpl.read/write and ServerSocket.implAccept cleanup diff -r a67ea4f53e56 -r 4503441bec2e src/java.base/share/classes/java/net/ServerSocket.java --- a/src/java.base/share/classes/java/net/ServerSocket.java Sun Feb 24 07:59:46 2019 +0000 +++ b/src/java.base/share/classes/java/net/ServerSocket.java Tue Feb 26 08:53:16 2019 +0000 @@ -541,52 +541,32 @@ * @spec JSR-51 */ protected final void implAccept(Socket s) throws IOException { - SocketImpl impl = getImpl(); SocketImpl si = s.impl; - // Socket does not have a SocketImpl + // Socket has no SocketImpl if (si == null) { // create a SocketImpl and accept the connection si = Socket.createImpl(); - assert !(si instanceof DelegatingSocketImpl); - impl.accept(si); - try { - // a custom impl has accepted the connection with a platform SocketImpl - if (!(impl instanceof PlatformSocketImpl) && (si instanceof PlatformSocketImpl)) { - ((PlatformSocketImpl) si).postCustomAccept(); - } - } finally { - securityCheckAccept(si); // closes si if permission check fails - } - + implAccept(si); // bind Socket to the SocketImpl and update socket state s.setImpl(si); s.postAccept(); return; } - // Socket has a SOCKS or HTTP SocketImpl + // Socket has a SOCKS or HTTP SocketImpl, need delegate if (si instanceof DelegatingSocketImpl) { si = ((DelegatingSocketImpl) si).delegate(); assert si instanceof PlatformSocketImpl; } - // ServerSocket or Socket (or both) have a platform SocketImpl + // ServerSocket or Socket (or both) have, or delegate to, a platform SocketImpl if (impl instanceof PlatformSocketImpl || si instanceof PlatformSocketImpl) { // create a new platform SocketImpl and accept the connection - var nsi = SocketImpl.createPlatformSocketImpl(false); - impl.accept(nsi); - try { - // a custom impl has accepted the connection - if (!(impl instanceof PlatformSocketImpl)) { - nsi.postCustomAccept(); - } - } finally { - securityCheckAccept(nsi); // closes nsi if permission check fails - } - - // copy state to the existing SocketImpl and update socket state - nsi.copyTo(si); + var psi = SocketImpl.createPlatformSocketImpl(false); + implAccept(psi); + // copy connection/state to the existing SocketImpl and update socket state + psi.copyTo(si); s.postAccept(); return; } @@ -610,6 +590,26 @@ } /** + * Accepts a new connection so that the given SocketImpl is connected to + * the peer. The SocketImpl and connection are closed if the connection is + * denied by the security manager. + * @throws IOException if an I/O error occurs + * @throws SecurityException if the security manager's checkAccept method fails + */ + private void implAccept(SocketImpl si) throws IOException { + assert !(si instanceof DelegatingSocketImpl); + impl.accept(si); + try { + // a custom impl has accepted the connection with a platform SocketImpl + if (!(impl instanceof PlatformSocketImpl) && (si instanceof PlatformSocketImpl)) { + ((PlatformSocketImpl) si).postCustomAccept(); + } + } finally { + securityCheckAccept(si); // closes si if permission check fails + } + } + + /** * Invokes the security manager's checkAccept method. If the permission * check fails then it closes the SocketImpl. */ diff -r a67ea4f53e56 -r 4503441bec2e src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java --- a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java Sun Feb 24 07:59:46 2019 +0000 +++ b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java Tue Feb 26 08:53:16 2019 +0000 @@ -115,10 +115,10 @@ private long readerThread; private long writerThread; - // used when SO_REUSEADDR is emulated + // used when SO_REUSEADDR is emulated, protected by stateLock private boolean isReuseAddress; - // cached value of IPV6_TCLASS or IP_TOS socket option + // cached value of IPV6_TCLASS or IP_TOS socket option, protected by stateLock private int trafficClass; // read or accept timeout in millis @@ -128,6 +128,10 @@ private volatile boolean isInputClosed; private volatile boolean isOutputClosed; + // used by read to emulate legacy behavior, protected by readLock + private boolean readEOF; + private boolean connectionReset; + /** * Creates a instance of this SocketImpl. * @param server true if this is a SocketImpl for a ServerSocket @@ -254,6 +258,7 @@ private int timedRead(FileDescriptor fd, byte[] b, int off, int len, int millis) throws IOException { + assert nonBlocking; long nanos = NANOSECONDS.convert(millis, TimeUnit.MILLISECONDS); long remainingNanos = nanos; long startNanos = System.nanoTime(); @@ -273,18 +278,23 @@ /** * Reads bytes from the socket into the given byte array. - * @return the number of bytes read - * @throws IOException if the socket is closed or an I/O occurs + * @return the number of bytes read or -1 at EOF + * @throws SocketException if the socket is closed or a socket I/O error occurs * @throws SocketTimeoutException if the read timeout elapses */ - private int read(byte[] b, int off, int len) throws IOException { + private int implRead(byte[] b, int off, int len) throws IOException { readLock.lock(); try { + // emulate legacy behavior to return -1, even if socket is closed + if (readEOF) + return -1; int n = 0; FileDescriptor fd = beginRead(); try { + if (connectionReset) + throw new SocketException("Connection reset"); if (isInputClosed) - return IOStatus.EOF; + return -1; int timeout = this.timeout; configureNonBlockingIfNeeded(fd, timeout); n = tryRead(fd, b, off, len); @@ -300,7 +310,16 @@ } while (IOStatus.okayToRetry(n) && isOpen()); } } + if (n == -1) + readEOF = true; return n; + } catch (SocketTimeoutException e) { + throw e; + } catch (ConnectionResetException e) { + connectionReset = true; + throw new SocketException("Connection reset"); + } catch (IOException ioe) { + throw new SocketException(ioe.getMessage()); } finally { endRead(n > 0); } @@ -310,6 +329,24 @@ } /** + * Reads bytes from the socket into the given byte array. + * @return the number of bytes read or -1 at EOF + * @throws IndexOutOfBoundsException if the bound checks fail + * @throws SocketException if the socket is closed or a socket I/O error occurs + * @throws SocketTimeoutException if the read timeout elapses + */ + private int read(byte[] b, int off, int len) throws IOException { + Objects.checkFromIndexSize(off, len, b.length); + if (len == 0) { + return 0; + } else { + // read up to MAX_BUFFER_SIZE bytes + int size = Math.min(len, MAX_BUFFER_SIZE); + return implRead(b, off, size); + } + } + + /** * Marks the beginning of a write operation that might block. * @throws SocketException if the socket is closed or not connected */ @@ -355,9 +392,9 @@ /** * Writes a sequence of bytes to this socket from the given byte array. * @return the number of bytes written - * @throws IOException if the socket is closed or an I/O occurs + * @throws SocketException if the socket is closed or an socket I/O error occurs */ - private int write(byte[] b, int off, int len) throws IOException { + private int implWrite(byte[] b, int off, int len) throws IOException { writeLock.lock(); try { int n = 0; @@ -369,6 +406,8 @@ n = tryWrite(fd, b, off, len); } return n; + } catch (IOException ioe) { + throw new SocketException(ioe.getMessage()); } finally { endWrite(n > 0); } @@ -378,6 +417,24 @@ } /** + * Writes a sequence of bytes to this socket from the given byte array. + * @throws SocketException if the socket is closed or an socket I/O error occurs + */ + private void write(byte[] b, int off, int len) throws IOException { + Objects.checkFromIndexSize(off, len, b.length); + if (len > 0) { + int pos = off; + int end = off + len; + while (pos < end) { + // write up to MAX_BUFFER_SIZE bytes + int size = Math.min((end - pos), MAX_BUFFER_SIZE); + int n = implWrite(b, pos, size); + pos += n; + } + } + } + + /** * Creates the socket. * @param stream {@code true} for a streams socket */ @@ -440,6 +497,7 @@ nsi.close(); } catch (IOException ignore) { } } + // copy/reset fields protected by stateLock synchronized (nsi.stateLock) { assert nsi.state == ST_NEW || nsi.state == ST_CLOSED; synchronized (this.stateLock) { @@ -456,9 +514,9 @@ // reset fields; do not reset timeout nsi.nonBlocking = false; + nsi.isReuseAddress = false; nsi.isInputClosed = false; nsi.isOutputClosed = false; - nsi.isReuseAddress = false; nsi.state = ST_CONNECTED; // GC'ing of this impl should not close the file descriptor @@ -469,6 +527,14 @@ nsi.closer = FileDescriptorCloser.create(nsi); } } + // reset fields protected by readLock + nsi.readLock.lock(); + try { + nsi.readEOF = false; + nsi.connectionReset = false; + } finally { + nsi.readLock.unlock(); + } } else { synchronized (this.stateLock) { // this SocketImpl should be connected @@ -696,6 +762,7 @@ int millis) throws IOException { + assert nonBlocking; long nanos = NANOSECONDS.convert(millis, TimeUnit.MILLISECONDS); long remainingNanos = nanos; long startNanos = System.nanoTime(); @@ -783,8 +850,6 @@ @Override protected InputStream getInputStream() { return new InputStream() { - // EOF or connection reset detected, not thread safe - private boolean eof, reset; @Override public int read() throws IOException { byte[] a = new byte[1]; @@ -793,31 +858,7 @@ } @Override public int read(byte[] b, int off, int len) throws IOException { - Objects.checkFromIndexSize(off, len, b.length); - if (eof) { - return -1; - } else if (reset) { - NioSocketImpl.this.ensureOpen(); - throw new SocketException("Connection reset"); - } else if (len == 0) { - return 0; - } else { - try { - // read up to MAX_BUFFER_SIZE bytes - int size = Math.min(len, MAX_BUFFER_SIZE); - int n = NioSocketImpl.this.read(b, off, size); - if (n == -1) - eof = true; - return n; - } catch (ConnectionResetException e) { - reset = true; - throw new SocketException("Connection reset"); - } catch (SocketTimeoutException e) { - throw e; - } catch (IOException ioe) { - throw new SocketException(ioe.getMessage()); - } - } + return NioSocketImpl.this.read(b, off, len); } @Override public int available() throws IOException { @@ -840,23 +881,8 @@ } @Override public void write(byte[] b, int off, int len) throws IOException { - Objects.checkFromIndexSize(off, len, b.length); - if (len > 0) { - try { - int pos = off; - int end = off + len; - while (pos < end) { - // write up to MAX_BUFFER_SIZE bytes - int size = Math.min((end - pos), MAX_BUFFER_SIZE); - int n = NioSocketImpl.this.write(b, pos, size); - pos += n; - } - } catch (IOException ioe) { - throw new SocketException(ioe.getMessage()); - } - } + NioSocketImpl.this.write(b, off, len); } - @Override public void close() throws IOException { NioSocketImpl.this.close();