changeset 54317:71bd81aad49f fibers

Sync NioSocketImpl with sandbox
author alanb
date Thu, 14 Mar 2019 12:42:07 +0000
parents d4949ac33b47
children c3e1c6edebac
files src/java.base/share/classes/java/net/ServerSocket.java src/java.base/share/classes/java/net/SocketImpl.java src/java.base/share/classes/sun/net/PlatformSocketImpl.java src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java
diffstat 4 files changed, 102 insertions(+), 137 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/net/ServerSocket.java	Mon Mar 11 07:00:48 2019 +0000
+++ b/src/java.base/share/classes/java/net/ServerSocket.java	Thu Mar 14 12:42:07 2019 +0000
@@ -546,15 +546,7 @@
 
         // Socket has no SocketImpl
         if (si == null) {
-            // create a platform or custom SocketImpl and accept the connection
-            SocketImplFactory factory = Socket.socketImplFactory();
-            if (factory == null) {
-                si = SocketImpl.createPlatformSocketImpl(false);
-            } else {
-                si = factory.createSocketImpl();
-            }
-            implAccept(si);
-            // bind Socket to the SocketImpl and update socket state
+            si = implAccept();
             s.setImpl(si);
             s.postAccept();
             return;
@@ -566,29 +558,82 @@
             assert si instanceof PlatformSocketImpl;
         }
 
-        // 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 psi = SocketImpl.createPlatformSocketImpl(false);
-            implAccept(psi);
-            // copy connection/state to the existing SocketImpl and update socket state
-            psi.copyTo(si);
-            s.postAccept();
-            return;
+        // Accept connection with a platform or custom SocketImpl.
+        // For the platform SocketImpl case:
+        // - the connection is accepted with a new SocketImpl
+        // - the SO_TIMEOUT socket option is copied to the new SocketImpl
+        // - the Socket is connected to the new SocketImpl
+        // - the existing/old SocketImpl is closed
+        // For the custom SocketImpl case, the connection is accepted with the
+        // existing custom SocketImpl.
+        ensureCompatible(si);
+        if (impl instanceof PlatformSocketImpl) {
+            SocketImpl psi = platformImplAccept();
+            si.copyOptionsTo(psi);
+            s.setImpl(psi);
+            si.closeQuietly();
+        } else {
+            s.impl = null; // temporarily break connection to impl
+            try {
+                customImplAccept(si);
+            } finally {
+                s.impl = si;  // restore connection to impl
+            }
         }
+        s.postAccept();
+    }
 
-        // ServerSocket and Socket bound to custom SocketImpls
-        s.impl = null; // break connection to impl
+    /**
+     * Accepts a connection with a new SocketImpl.
+     * @return the new SocketImpl
+     */
+    private SocketImpl implAccept() throws IOException {
+        if (impl instanceof PlatformSocketImpl) {
+            return platformImplAccept();
+        } else {
+            // custom server SocketImpl, client SocketImplFactory must be set
+            SocketImplFactory factory = Socket.socketImplFactory();
+            if (factory == null) {
+                throw new IOException("An instance of " + impl.getClass() +
+                    " cannot accept connection with 'null' SocketImpl:" +
+                    " client socket implementation factory not set");
+            }
+            SocketImpl si = factory.createSocketImpl();
+            customImplAccept(si);
+            return si;
+        }
+    }
+
+    /**
+     * Accepts a connection with a new platform SocketImpl.
+     * @return the new platform SocketImpl
+     */
+    private SocketImpl platformImplAccept() throws IOException {
+        assert impl instanceof PlatformSocketImpl;
+
+        // create a new platform SocketImpl and accept the connection
+        SocketImpl psi = SocketImpl.createPlatformSocketImpl(false);
+        implAccept(psi);
+        return psi;
+    }
+
+    /**
+     * Accepts a new connection with the given custom SocketImpl.
+     */
+    private void customImplAccept(SocketImpl si) throws IOException {
+        assert !(impl instanceof PlatformSocketImpl)
+                && !(si instanceof PlatformSocketImpl);
+
         si.reset();
         try {
+            // custom SocketImpl may expect fd/address objects to be created
+            si.fd = new FileDescriptor();
+            si.address = new InetAddress();
             implAccept(si);
         } catch (Exception e) {
             si.reset();
             throw e;
-        } finally {
-            s.impl = si;  // restore connection to impl
         }
-        s.postAccept();
     }
 
     /**
@@ -601,30 +646,9 @@
     private void implAccept(SocketImpl si) throws IOException {
         assert !(si instanceof DelegatingSocketImpl);
 
-        // A non-platform SocketImpl cannot accept a connection with a platform SocketImpl
-        if (!(impl instanceof PlatformSocketImpl) && (si instanceof PlatformSocketImpl)) {
-            throw new IOException("An instance of " + impl.getClass() +
-                " cannot accept a connection with an instance of " + si.getClass());
-        }
-
-        // custom SocketImpl may expect fd/address objects to be created
-        if (!(si instanceof PlatformSocketImpl)) {
-            si.fd = new FileDescriptor();
-            si.address = new InetAddress();
-        }
-
         // accept a connection
         impl.accept(si);
 
-        // sanity check that the fields defined by SocketImpl have been set
-        if (si instanceof PlatformSocketImpl) {
-            var fd = si.fd;
-            if (fd == null || !fd.valid() || si.localport <= 0
-                    || si.address == null || si.port <= 0) {
-                throw new IOException("Invalid accepted state:" + si);
-            }
-        }
-
         // check permission, close SocketImpl/connection if denied
         SecurityManager sm = System.getSecurityManager();
         if (sm != null) {
@@ -638,6 +662,17 @@
     }
 
     /**
+     * Throws IOException if the server SocketImpl and the given client
+     * SocketImpl are not both platform or custom SocketImpls.
+     */
+    private void ensureCompatible(SocketImpl si) throws IOException {
+        if ((impl instanceof PlatformSocketImpl) != (si instanceof PlatformSocketImpl)) {
+            throw new IOException("An instance of " + impl.getClass() +
+                " cannot accept a connection with an instance of " + si.getClass());
+        }
+    }
+
+    /**
      * Closes this socket.
      *
      * Any thread currently blocked in {@link #accept()} will throw
--- a/src/java.base/share/classes/java/net/SocketImpl.java	Mon Mar 11 07:00:48 2019 +0000
+++ b/src/java.base/share/classes/java/net/SocketImpl.java	Thu Mar 14 12:42:07 2019 +0000
@@ -190,6 +190,15 @@
     protected abstract void close() throws IOException;
 
     /**
+     * Closes this socket, ignoring any IOException that is thrown by close.
+     */
+    void closeQuietly() {
+        try {
+            close();
+        } catch (IOException ignore) { }
+    }
+
+    /**
      * Places the input stream for this socket at "end of stream".
      * Any data sent to this socket is acknowledged and then
      * silently discarded.
@@ -319,6 +328,7 @@
     }
 
     void reset() throws IOException {
+        fd = null;
         address = null;
         port = 0;
         localport = 0;
@@ -455,6 +465,19 @@
         }
     }
 
+    /**
+     * Attempts to copy socket options from this SocketImpl to a target SocketImpl.
+     * At this time, only the SO_TIMEOUT make sense to copy.
+     */
+    void copyOptionsTo(SocketImpl target) {
+        try {
+            Object timeout = getOption(SocketOptions.SO_TIMEOUT);
+            if (timeout instanceof Integer) {
+                target.setOption(SocketOptions.SO_TIMEOUT, timeout);
+            }
+        } catch (IOException ignore) { }
+    }
+
     private static final Set<SocketOption<?>> socketOptions;
 
     private static final Set<SocketOption<?>> serverSocketOptions;
--- a/src/java.base/share/classes/sun/net/PlatformSocketImpl.java	Mon Mar 11 07:00:48 2019 +0000
+++ b/src/java.base/share/classes/sun/net/PlatformSocketImpl.java	Thu Mar 14 12:42:07 2019 +0000
@@ -24,20 +24,9 @@
  */
 package sun.net;
 
-import java.net.SocketImpl;
-
 /**
  * Implemented by the platform's SocketImpl implementations.
  */
 
 public interface PlatformSocketImpl {
-
-    /**
-     * Copy the state from this connected SocketImpl to a target SocketImpl. If
-     * the target SocketImpl is not a newly created SocketImpl then it is first
-     * closed to release any resources. The target SocketImpl becomes the owner
-     * of the file descriptor, this SocketImpl is marked as closed and should
-     * be discarded.
-     */
-    void copyTo(SocketImpl si);
 }
--- a/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java	Mon Mar 11 07:00:48 2019 +0000
+++ b/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java	Thu Mar 14 12:42:07 2019 +0000
@@ -500,77 +500,6 @@
     }
 
     /**
-     * For use by ServerSocket to copy the state from this connected SocketImpl
-     * to a target SocketImpl. If the target SocketImpl is not a newly created
-     * SocketImpl then it is first closed to release any resources. The target
-     * SocketImpl becomes the owner of the file descriptor, this SocketImpl
-     * is marked as closed and should be discarded.
-     */
-    @Override
-    public void copyTo(SocketImpl si) {
-        if (si instanceof NioSocketImpl) {
-            NioSocketImpl nsi = (NioSocketImpl) si;
-            if (nsi.state != ST_NEW) {
-                try {
-                    nsi.close();
-                } catch (IOException ignore) { }
-            }
-            // copy/reset fields protected by stateLock
-            synchronized (nsi.stateLock) {
-                guarantee(nsi.state == ST_NEW || nsi.state == ST_CLOSED);
-                synchronized (this.stateLock) {
-                    // this SocketImpl should be connected
-                    guarantee(state == ST_CONNECTED && fd != null && fd.valid()
-                        && localport > 0 && address != null && port > 0);
-
-                    // copy fields
-                    nsi.stream = this.stream;
-                    nsi.fd = this.fd;
-                    nsi.localport = this.localport;
-                    nsi.address = this.address;
-                    nsi.port = this.port;
-
-                    // reset fields; do not reset timeout
-                    nsi.nonBlocking = false;
-                    nsi.isReuseAddress = false;
-                    nsi.isInputClosed = false;
-                    nsi.isOutputClosed = false;
-                    nsi.state = ST_CONNECTED;
-
-                    // GC'ing of this impl should not close the file descriptor
-                    this.closer.disable();
-                    this.state = ST_CLOSED;
-
-                    // create new closer to execute when nsi is GC'ed
-                    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
-                guarantee(state == ST_CONNECTED && fd != null && fd.valid()
-                        && localport > 0 && address != null && port > 0);
-
-                // set fields in foreign impl
-                setSocketImplFields(si, fd, localport, address, port);
-
-                // disable closer to prevent GC'ing of this impl from
-                // closing the file descriptor
-                this.closer.disable();
-                this.state = ST_CLOSED;
-            }
-        }
-    }
-
-    /**
      * Marks the beginning of a connect operation that might block.
      * @throws SocketException if the socket is closed or already connected
      */
@@ -1322,10 +1251,6 @@
                 }
             }
         }
-
-        boolean disable() {
-            return CLOSED.compareAndSet(this, false, true);
-        }
     }
 
     /**
@@ -1378,11 +1303,4 @@
         field.setAccessible(true);
         field.set(si, value);
     }
-
-    /**
-     * Throws InternalError if the given expression is not true.
-     */
-    private static void guarantee(boolean expr) {
-        if (!expr) throw new InternalError();
-    }
 }