Fix issues when configured to use PlainSocketImpl niosocketimpl-branch
authoralanb
Sat, 09 Feb 2019 19:16:30 +0000
branchniosocketimpl-branch
changeset 57171 d8ed7335dadd
parent 57170 52bc64277201
child 57172 63ab5af5d009
Fix issues when configured to use PlainSocketImpl
src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java
src/java.base/share/classes/java/net/HttpConnectSocketImpl.java
src/java.base/share/classes/java/net/ServerSocket.java
src/java.base/share/classes/java/net/Socket.java
src/java.base/share/classes/java/net/SocketImpl.java
src/java.base/share/classes/java/net/SocksSocketImpl.java
--- a/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java	Sat Feb 09 08:54:02 2019 +0000
+++ b/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java	Sat Feb 09 19:16:30 2019 +0000
@@ -453,15 +453,17 @@
 
     /**
      * Accepts connections.
-     * @param s the connection
+     * @param si the socket impl
      */
-    protected void accept(SocketImpl s) throws IOException {
+    protected void accept(SocketImpl si) throws IOException {
+        si.fd = new FileDescriptor();
         acquireFD();
         try {
-            socketAccept(s);
+            socketAccept(si);
         } finally {
             releaseFD();
         }
+        SocketCleanable.register(si.fd);
     }
 
     /**
@@ -729,31 +731,28 @@
         socketClose0(false);
     }
 
-    void postCustomAccept() {
-        // defaults ok
-    }
+    /**
+     * For use by ServerSocket to copy the state from this connected SocketImpl
+     * to a target SocketImpl.
+     */
+    void copyTo(SocketImpl si) {
+        if (si instanceof AbstractPlainSocketImpl) {
+            try {
+                si.close();
+            } catch (IOException ignore) { }
 
-    void copyTo(SocketImpl dest) {
-        if (dest instanceof PlainSocketImpl) {
-            AbstractPlainSocketImpl psi = (AbstractPlainSocketImpl)dest;
-            try {
-                dest.close();
-            } catch (IOException e) {}
+            // this SocketImpl should be connected
+            assert fd.valid() && localport != 0 && address != null && port != 0;
+
+            AbstractPlainSocketImpl psi = (AbstractPlainSocketImpl) si;
+            psi.stream = this.stream;
             psi.fd = this.fd;
-            psi.socket = this.socket;
-            psi.serverSocket = this.serverSocket;
+            psi.closePending = false;
+            psi.localport = this.localport;
             psi.address = this.address;
             psi.port = this.port;
-            psi.localport = this.localport;
-            psi.trafficClass = this.trafficClass;
-            psi.socketInputStream = this.socketInputStream;
-            psi.socketOutputStream = this.socketOutputStream;
-            psi.fdUseCount = this.fdUseCount;
-            psi.stream = this.stream;
-            psi.connectionReset = this.connectionReset;
-            psi.closePending = this.closePending;
-            psi.shut_rd = this.shut_rd;
-            psi.shut_wr = this.shut_wr;
+        } else {
+            throw new RuntimeException("not implemented");
         }
     }
 
--- a/src/java.base/share/classes/java/net/HttpConnectSocketImpl.java	Sat Feb 09 08:54:02 2019 +0000
+++ b/src/java.base/share/classes/java/net/HttpConnectSocketImpl.java	Sat Feb 09 19:16:30 2019 +0000
@@ -330,18 +330,4 @@
     public SocketImpl delegate() {
         return delegate;
     }
-
-    @Override
-    public SocketImpl newInstance() {
-        if (delegate instanceof PlainSocketImpl)
-            return new HttpConnectSocketImpl(new PlainSocketImpl());
-        else if (delegate instanceof NioSocketImpl)
-            return new HttpConnectSocketImpl(new NioSocketImpl(false));
-        throw new InternalError();
-    }
-
-    @Override
-    public void postCustomAccept() {
-        // TODO
-    }
 }
--- a/src/java.base/share/classes/java/net/ServerSocket.java	Sat Feb 09 08:54:02 2019 +0000
+++ b/src/java.base/share/classes/java/net/ServerSocket.java	Sat Feb 09 19:16:30 2019 +0000
@@ -38,8 +38,6 @@
 import java.util.Set;
 import java.util.Collections;
 import sun.nio.ch.NioSocketImpl;
-import static java.net.SocketImpl.DelegatingImpl;
-import static java.net.SocketImpl.getDefaultSocketImpl;
 
 /**
  * This class implements server sockets. A server socket waits for
@@ -299,7 +297,7 @@
         } else {
             // No need to do a checkOldImpl() here, we know it's an up to date
             // SocketImpl!
-            impl = new SocksSocketImpl(getDefaultSocketImpl(true));
+            impl = SocketImpl.createDefaultSocketImpl(true);
         }
         if (impl != null)
             impl.setServerSocket(this);
@@ -528,23 +526,6 @@
         return s;
     }
 
-    static void prepareImpl(SocketImpl impl) {
-        if (!(impl instanceof DelegatingImpl))
-            return;
-        impl = ((DelegatingImpl)impl).delegate();
-        if (impl.address == null)
-            impl.address = new InetAddress();
-        if (impl.fd == null)
-            impl.fd = new FileDescriptor();
-    }
-
-    static SocketImpl newInstanceOfType(SocketImpl impl1, SocketImpl impl2) {
-        if (impl1 instanceof DelegatingImpl)
-            return ((DelegatingImpl)impl1).newInstance();
-        else
-            return ((DelegatingImpl)impl2).newInstance();
-    }
-
     /**
      * Subclasses of ServerSocket use this method to override accept()
      * to return their own subclass of socket.  So a FooServerSocket
@@ -569,14 +550,11 @@
         if (si == null) {
             // create a SocketImpl and accept the connection
             si = Socket.createImpl();
-            prepareImpl(si);
-            s.setImpl(si);
             impl.accept(si);
-
             try {
-                // a custom impl has accepted the connection with a Platform SocketImpl
-                if (!(impl instanceof DelegatingImpl) && (si instanceof DelegatingImpl)) {
-                    ((DelegatingImpl)si).postCustomAccept();
+                // a custom impl has accepted the connection with a NIO SocketImpl
+                if ((si instanceof NioSocketImpl) && !(impl instanceof NioSocketImpl)) {
+                    ((NioSocketImpl) si).postCustomAccept();
                 }
             } finally {
                 securityCheckAccept(si);  // closes si if permission check fails
@@ -588,18 +566,31 @@
             return;
         }
 
-        // ServerSocket or Socket is using NIO or Plain SocketImpl
-        if (impl instanceof DelegatingImpl || si instanceof DelegatingImpl) {
+        if (si instanceof SocketImpl.DelegatingImpl)
+            si = ((SocketImpl.DelegatingImpl) si).delegate();
 
+        // ServerSocket or Socket is using NIO SocketImpl
+        if (impl instanceof NioSocketImpl || si instanceof NioSocketImpl) {
             // accept connection via new SocketImpl
-
-            SocketImpl nsi = newInstanceOfType(impl, si);
-            prepareImpl(nsi);
+            NioSocketImpl nsi = new NioSocketImpl(false);
             impl.accept(nsi);
             securityCheckAccept(nsi);  // closes si if permission check fails
 
             // copy state to the existing SocketImpl and update socket state
-            ((DelegatingImpl)nsi).copyTo(si);
+            nsi.copyTo(si);
+            s.postAccept();
+            return;
+        }
+
+        // ServerSocket or Socket is using PlainSocketImpl
+        if (impl instanceof PlainSocketImpl || si instanceof PlainSocketImpl) {
+            // accept connection via new SocketImpl
+            PlainSocketImpl psi = new PlainSocketImpl();
+            impl.accept(psi);
+            securityCheckAccept(psi);  // closes si if permission check fails
+
+            // copy state to the existing SocketImpl and update socket state
+            psi.copyTo(si);
             s.postAccept();
             return;
         }
@@ -609,7 +600,8 @@
         boolean completed = false;
         try {
             si.reset();
-            prepareImpl(si);
+            si.fd = new FileDescriptor();
+            si.address = new InetAddress();
             impl.accept(si);
             securityCheckAccept(si);  // closes si if permission check fails
             completed = true;
--- a/src/java.base/share/classes/java/net/Socket.java	Sat Feb 09 08:54:02 2019 +0000
+++ b/src/java.base/share/classes/java/net/Socket.java	Sat Feb 09 19:16:30 2019 +0000
@@ -34,8 +34,6 @@
 import java.util.Set;
 import java.util.Collections;
 
-import static java.net.SocketImpl.getDefaultSocketImpl;
-
 /**
  * This class implements client sockets (also called just
  * "sockets"). A socket is an endpoint for communication
@@ -138,14 +136,18 @@
                     security.checkConnect(epoint.getAddress().getHostAddress(),
                                   epoint.getPort());
             }
-            impl = type == Proxy.Type.SOCKS 
-                ? new SocksSocketImpl(p, getDefaultSocketImpl(false))
-	            : new HttpConnectSocketImpl(p, getDefaultSocketImpl(false));
+
+            SocketImpl si = SocketImpl.createDefaultSocketImpl(false);
+            if (type == Proxy.Type.SOCKS) {
+                impl = new SocksSocketImpl(p, si);
+            } else {
+                impl = new HttpConnectSocketImpl(p, si);
+            }
             impl.setSocket(this);
         } else {
             if (p == Proxy.NO_PROXY) {
                 if (factory == null) {
-                    impl = new SocksSocketImpl(getDefaultSocketImpl(false));
+                    impl = SocketImpl.createDefaultSocketImpl(false);
                     impl.setSocket(this);
                 } else
                     setImpl();
@@ -498,7 +500,7 @@
         if (factory != null) {
             return factory.createSocketImpl();
         } else {
-            return new SocksSocketImpl(getDefaultSocketImpl(false));
+            return SocketImpl.createDefaultSocketImpl(false);
         }
     }
 
@@ -518,7 +520,8 @@
         } else {
             // No need to do a checkOldImpl() here, we know it's an up to date
             // SocketImpl!
-            impl = new SocksSocketImpl(getDefaultSocketImpl(false));
+            SocketImpl si = SocketImpl.createDefaultSocketImpl(false);
+            impl = new SocksSocketImpl(si);
         }
         if (impl != null)
             impl.setSocket(this);
--- a/src/java.base/share/classes/java/net/SocketImpl.java	Sat Feb 09 08:54:02 2019 +0000
+++ b/src/java.base/share/classes/java/net/SocketImpl.java	Sat Feb 09 19:16:30 2019 +0000
@@ -72,7 +72,7 @@
      */
     protected int localport;
 
-    private static boolean useNioSocketImpl = getUseNioSocketImpl();
+    private static final boolean useNioSocketImpl = getUseNioSocketImpl();
 
     // A simple way to override the socketimpl by creating a file in $user.dir
     private static boolean getUseNioSocketImpl() {
@@ -90,7 +90,7 @@
         }
     }
 
-    static SocketImpl getDefaultSocketImpl(boolean server) {
+    static SocketImpl createDefaultSocketImpl(boolean server) {
         if (useNioSocketImpl) {
             return new NioSocketImpl(server);
         } else {
@@ -98,7 +98,6 @@
         }
     }
 
-
     /**
      * Creates either a stream or a datagram socket.
      *
@@ -332,20 +331,6 @@
      */
     interface DelegatingImpl {
         SocketImpl delegate();
-        SocketImpl newInstance();
-        void postCustomAccept();
-
-        default void copyTo(SocketImpl dest) {
-            SocketImpl src = delegate();
-            if (dest instanceof DelegatingImpl)
-                dest = ((DelegatingImpl)dest).delegate();
-            if (src instanceof NioSocketImpl) {
-                ((NioSocketImpl)src).copyTo(dest);
-            } else if (src instanceof PlainSocketImpl) {
-                ((PlainSocketImpl)src).copyTo(dest);
-            } else
-                throw new InternalError();
-        }
     }
 
     /**
--- a/src/java.base/share/classes/java/net/SocksSocketImpl.java	Sat Feb 09 08:54:02 2019 +0000
+++ b/src/java.base/share/classes/java/net/SocksSocketImpl.java	Sat Feb 09 19:16:30 2019 +0000
@@ -52,7 +52,7 @@
     private Socket cmdsock = null;
     private InputStream cmdIn = null;
     private OutputStream cmdOut = null;
-    final SocketImpl delegate;
+    private final SocketImpl delegate;
 
     SocksSocketImpl(SocketImpl delegate) {
         Objects.requireNonNull(delegate);
@@ -692,23 +692,4 @@
     public SocketImpl delegate() {
         return delegate;
     }
-
-    @Override
-    public SocketImpl newInstance() {
-        if (delegate instanceof PlainSocketImpl)
-            return new SocksSocketImpl(new PlainSocketImpl());
-        else if (delegate instanceof NioSocketImpl)
-            return new SocksSocketImpl(new NioSocketImpl(false));
-        throw new InternalError();
-    }
-
-    @Override
-    public void postCustomAccept() {
-        if (delegate instanceof NioSocketImpl) {
-            // TODO ((NioSocketImpl)delegate).postCustomAccept();
-        } else if (delegate instanceof PlainSocketImpl) {
-            ((PlainSocketImpl)delegate).postCustomAccept();
-        } else
-            throw new InternalError();
-    }
 }