More cleanup of ServerSocket.accept niosocketimpl-branch
authoralanb
Tue, 12 Mar 2019 09:49:10 +0000
branchniosocketimpl-branch
changeset 57260 bb5198288772
parent 57252 d70fc9bc1430
child 57268 adcdd45830a0
More cleanup of ServerSocket.accept
src/java.base/share/classes/java/net/ServerSocket.java
src/java.base/share/classes/java/net/SocketImpl.java
--- a/src/java.base/share/classes/java/net/ServerSocket.java	Mon Mar 11 19:53:08 2019 +0000
+++ b/src/java.base/share/classes/java/net/ServerSocket.java	Tue Mar 12 09:49:10 2019 +0000
@@ -558,16 +558,27 @@
             assert si instanceof PlatformSocketImpl;
         }
 
-        // accept connection with a platform or custom SocketImpl
+        // 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();
-            tryCopyOptions(si, psi);
+            si.copyOptionsTo(psi);
             s.setImpl(psi);
-            close(si);
+            si.closeQuietly();
         } else {
-            customImplAccept(s, si);
-            assert s.impl == si;
+            s.impl = null; // temporarily break connection to impl
+            try {
+                customImplAccept(si);
+            } finally {
+                s.impl = si;  // restore connection to impl
+            }
         }
         s.postAccept();
     }
@@ -580,7 +591,7 @@
         if (impl instanceof PlatformSocketImpl) {
             return platformImplAccept();
         } else {
-            // custom server SocketImpl, client socket SocketImpl must be set
+            // custom server SocketImpl, client SocketImplFactory must be set
             SocketImplFactory factory = Socket.socketImplFactory();
             if (factory == null) {
                 throw new IOException("An instance of " + impl.getClass() +
@@ -588,7 +599,7 @@
                     " client socket implementation factory not set");
             }
             SocketImpl si = factory.createSocketImpl();
-            implAccept(si);
+            customImplAccept(si);
             return si;
         }
     }
@@ -607,21 +618,21 @@
     }
 
     /**
-     * Accepts a new connection with a custom SocketImpl.
+     * Accepts a new connection with the given custom SocketImpl.
      */
-    private void customImplAccept(Socket s, SocketImpl si) throws IOException {
+    private void customImplAccept(SocketImpl si) throws IOException {
         assert !(impl instanceof PlatformSocketImpl)
                 && !(si instanceof PlatformSocketImpl);
 
-        s.impl = null; // break connection to impl
         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
         }
     }
 
@@ -635,12 +646,6 @@
     private void implAccept(SocketImpl si) throws IOException {
         assert !(si instanceof DelegatingSocketImpl);
 
-        // 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);
 
@@ -657,28 +662,6 @@
     }
 
     /**
-     * Attempts to copy socket options from an old/existing SocketImpl to a new
-     * SocketImpl. At this time, only the SO_TIMEOUT make sense to copy.
-     */
-    private void tryCopyOptions(SocketImpl oldImpl, SocketImpl newImpl) {
-        try {
-            Object timeout = oldImpl.getOption(SocketOptions.SO_TIMEOUT);
-            if (timeout instanceof Integer) {
-                newImpl.setOption(SocketOptions.SO_TIMEOUT, timeout);
-            }
-        } catch (IOException ignore) { }
-    }
-
-    /**
-     * Close the given SocketImpl.
-     */
-    private void close(SocketImpl si) {
-        try {
-            si.close();
-        } catch (IOException ignore) { }
-    }
-
-    /**
      * Throws IOException if the server SocketImpl and the given client
      * SocketImpl are not both platform or custom SocketImpls.
      */
--- a/src/java.base/share/classes/java/net/SocketImpl.java	Mon Mar 11 19:53:08 2019 +0000
+++ b/src/java.base/share/classes/java/net/SocketImpl.java	Tue Mar 12 09:49:10 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1995, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -204,6 +204,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.
@@ -470,6 +479,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;