Remove legacy NIO double close

Closing the socket is not necessary. I found information on Java 1.4, 5 and 6 having possible issues that needed the socket close, but this is now fixed. NIO2 from Java 7 doesn't give the user a choice as its channel doesn't expose the socket to avoid abuse and bad practices.
apache · May 21, 2019 · e2d5a04 · e2d5a04
1 parent 8551008
commit e2d5a04
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 12 deletions.
diff --git a/java/org/apache/tomcat/util/net/NioChannel.java b/java/org/apache/tomcat/util/net/NioChannel.java
@@ -40,11 +40,10 @@ public class NioChannel implements ByteChannel, ScatteringByteChannel, Gathering
 
     protected static final ByteBuffer emptyBuf = ByteBuffer.allocate(0);
 
+    protected final SocketBufferHandler bufHandler;
     protected SocketChannel sc = null;
     protected NioSocketWrapper socketWrapper = null;
 
-    protected final SocketBufferHandler bufHandler;
-
     public NioChannel(SocketChannel channel, SocketBufferHandler bufHandler) {
         this.sc = channel;
         this.bufHandler = bufHandler;
@@ -102,7 +101,6 @@ public boolean flush(boolean block, Selector s, long timeout)
      */
     @Override
     public void close() throws IOException {
-        sc.socket().close();
         sc.close();
     }
 
@@ -114,7 +112,9 @@ public void close() throws IOException {
      * @throws IOException If closing the secure channel fails.
      */
     public void close(boolean force) throws IOException {
-        if (isOpen() || force ) close();
+        if (isOpen() || force) {
+            close();
+        }
     }
 
     /**

diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -350,7 +350,6 @@ public void unbind() throws Exception {
     protected void doCloseServerSocket() throws IOException {
         if (!getUseInheritedChannel() && serverSock != null) {
             // Close server socket
-            serverSock.socket().close();
             serverSock.close();
         }
         serverSock = null;
@@ -441,13 +440,6 @@ protected boolean setSocketOptions(SocketChannel socket) {
     @Override
     protected void closeSocket(SocketChannel socket) {
         countDownConnection();
-        try {
-            socket.socket().close();
-        } catch (IOException ioe)  {
-            if (log.isDebugEnabled()) {
-                log.debug(sm.getString("endpoint.err.close"), ioe);
-            }
-        }
         try {
             socket.close();
         } catch (IOException ioe) {

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -148,6 +148,10 @@
         Add support for same-site cookie attribute. Patch provided by John
         Kelly. (markt)
       </add>
+      <fix>
+        Drop legacy NIO double socket close (close channel, then close
+        socket). (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Cluster">