changeset 53288:c0b3cd105f73 fibers

NioSocketImpl cleanup/improvements
author alanb
date Thu, 10 Jan 2019 20:25:27 +0000
parents afc0109176f8
children 48f16f6a3805
files 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 test/jdk/ProblemList.txt
diffstat 4 files changed, 83 insertions(+), 54 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/net/ServerSocket.java	Thu Jan 10 14:01:14 2019 +0000
+++ b/src/java.base/share/classes/java/net/ServerSocket.java	Thu Jan 10 20:25:27 2019 +0000
@@ -543,37 +543,37 @@
      * @spec JSR-51
      */
     protected final void implAccept(Socket s) throws IOException {
-        SocketImpl si = null;
-        try {
-            if (s.impl == null)
-              s.setImpl();
-            else {
-                s.impl.reset();
+        // create a new impl for the new connection
+        SocketImpl si = Socket.createImpl();
+        getImpl().accept(si);
+        SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            try {
+                sm.checkAccept(si.getInetAddress().getHostAddress(), si.getPort());
+            } catch (SecurityException e) {
+                si.close();
+                throw e;
             }
-            si = s.impl;
-            s.impl = null;
-            si.address = new InetAddress();
-            si.fd = new FileDescriptor();
-            getImpl().accept(si);
+        }
 
-            SecurityManager security = System.getSecurityManager();
-            if (security != null) {
-                security.checkAccept(si.getInetAddress().getHostAddress(),
-                                     si.getPort());
+        // copy the timeout from the old impl to the new impl
+        SocketImpl previous = s.impl;
+        if (previous != null) {
+            int timeout = (int) previous.getOption(SocketOptions.SO_TIMEOUT);
+            if (timeout != 0) {
+                si.setOption(SocketOptions.SO_TIMEOUT, timeout);
             }
-        } catch (IOException e) {
-            if (si != null)
-                si.reset();
-            s.impl = si;
-            throw e;
-        } catch (SecurityException e) {
-            if (si != null)
-                si.reset();
-            s.impl = si;
-            throw e;
         }
-        s.impl = si;
+
+        // replace the impl, eagerly closing the old impl to ensure any
+        // resources are released
+        s.setImpl(si);
         s.postAccept();
+        if (previous != null) {
+            try {
+                previous.close();
+            } catch (IOException ignore) { }
+        }
     }
 
     /**
--- a/src/java.base/share/classes/java/net/Socket.java	Thu Jan 10 14:01:14 2019 +0000
+++ b/src/java.base/share/classes/java/net/Socket.java	Thu Jan 10 20:25:27 2019 +0000
@@ -492,6 +492,20 @@
         });
     }
 
+    static SocketImpl createImpl() {
+        SocketImplFactory factory = Socket.factory;
+        if (factory != null) {
+            return factory.createSocketImpl();
+        } else {
+            return new NioSocketImpl(false);
+        }
+    }
+
+    void setImpl(SocketImpl si) {
+         impl = si;
+         impl.setSocket(this);
+    }
+
     /**
      * Sets impl to the system-default type of SocketImpl.
      * @since 1.4
@@ -509,7 +523,6 @@
             impl.setSocket(this);
     }
 
-
     /**
      * Get the {@code SocketImpl} attached to this socket, creating
      * it if necessary.
--- a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java	Thu Jan 10 14:01:14 2019 +0000
+++ b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java	Thu Jan 10 20:25:27 2019 +0000
@@ -132,9 +132,9 @@
     private void ensureOpenAndConnected() throws SocketException {
         int state = this.state;
         if (state < ST_CONNECTED)
-            throw new SocketException("not connected");
+            throw new SocketException("Not connected");
         if (state > ST_CONNECTED)
-            throw new SocketException("socket closed");
+            throw new SocketException("Socket closed");
     }
 
     /**
@@ -529,14 +529,15 @@
             throw new UnsupportedOperationException("SocketImpl type not supported");
         NioSocketImpl si = (NioSocketImpl) obj;
 
+        // accept a connection
+        FileDescriptor newfd = new FileDescriptor();
+        InetSocketAddress[] isaa = new InetSocketAddress[1];
         readLock.lock();
         try {
-            InetSocketAddress[] isaa = new InetSocketAddress[1];
-
             int n = 0;
             try {
                 FileDescriptor fd = beginAccept();
-                n = ServerSocketChannelImpl.accept0(fd, si.fd, isaa);
+                n = ServerSocketChannelImpl.accept0(fd, newfd, isaa);
                 if (n == IOStatus.UNAVAILABLE && isOpen()) {
                     long nanos = NANOSECONDS.convert(timeout, TimeUnit.MILLISECONDS);
                     if (nanos > 0) {
@@ -544,7 +545,7 @@
                         do {
                             long startTime = System.nanoTime();
                             park(fd, Net.POLLIN, nanos);
-                            n = ServerSocketChannelImpl.accept0(fd, si.fd, isaa);
+                            n = ServerSocketChannelImpl.accept0(fd, newfd, isaa);
                             if (n == IOStatus.UNAVAILABLE) {
                                 nanos -= System.nanoTime() - startTime;
                                 if (nanos <= 0)
@@ -555,7 +556,7 @@
                         // accept, no timeout
                         do {
                             park(fd, Net.POLLIN, 0);
-                            n = ServerSocketChannelImpl.accept0(fd, si.fd, isaa);
+                            n = ServerSocketChannelImpl.accept0(fd, newfd, isaa);
                         } while (n == IOStatus.UNAVAILABLE && isOpen());
                     }
                 }
@@ -563,26 +564,31 @@
                 endAccept(n > 0);
                 assert IOStatus.check(n);
             }
-
-            // set fields in SocketImpl
-            synchronized (si.stateLock) {
-
-                try {
-                    IOUtil.configureBlocking(si.fd, false);
-                } catch (IOException ioe) {
-                    nd.close(si.fd);
-                    throw ioe;
-                }
-                si.stream = true;
-                si.closer = FileDescriptorCloser.create(si);
-                si.localport = Net.localAddress(si.fd).getPort();
-                si.address = isaa[0].getAddress();
-                si.port = isaa[0].getPort();
-                si.state = ST_CONNECTED;
-            }
         } finally {
             readLock.unlock();
         }
+
+        // set non-blocking and get local address
+        InetSocketAddress localAddress;
+        try {
+            localAddress = Net.localAddress(newfd);
+            IOUtil.configureBlocking(newfd, false);
+        } catch (IOException ioe) {
+            nd.close(newfd);
+            throw ioe;
+        }
+
+        // set the fields
+        InetSocketAddress remoteAddress = isaa[0];
+        synchronized (si.stateLock) {
+            si.fd = newfd;
+            si.stream = true;
+            si.closer = FileDescriptorCloser.create(si);
+            si.localport = localAddress.getPort();
+            si.address = remoteAddress.getAddress();
+            si.port = remoteAddress.getPort();
+            si.state = ST_CONNECTED;
+        }
     }
 
     @Override
@@ -671,9 +677,16 @@
         boolean interrupted = false;
 
         synchronized (stateLock) {
-            if (state > ST_CONNECTED)
+            int state = this.state;
+            if (state >= ST_CLOSING)
                 return;
-            state = ST_CLOSING;
+            if (state == ST_NEW) {
+                // stillborn
+                this.state = ST_CLOSED;
+                return;
+            }
+            this.state = ST_CLOSING;
+            assert fd != null && closer != null;
 
             // unpark and wait for fibers to complete I/O operations
             if (NativeThread.isFiber(readerThread) ||
@@ -717,7 +730,7 @@
             try {
                 closer.run();
             } finally {
-                state = ST_CLOSED;
+                this.state = ST_CLOSED;
             }
         }
 
--- a/test/jdk/ProblemList.txt	Thu Jan 10 14:01:14 2019 +0000
+++ b/test/jdk/ProblemList.txt	Thu Jan 10 20:25:27 2019 +0000
@@ -572,6 +572,9 @@
 java/net/Socks/SocksIPv6Test.java                               0000000 generic-all
 java/net/Socket/SocksConnectTimeout.java                        0000000 generic-all
 
+java/net/Socket/HttpProxy.java                                  0000000 generic-all
+java/net/PlainSocketImpl/CustomSocketImplFactory.java	        0000000 generic-all
+
 java/net/Inet6Address/B6206527.java                             8216417 macosx-all
 java/net/ipv6tests/B6521014.java                                8216417 macosx-all