You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/05/29 13:11:28 UTC

[tomcat] branch master updated: Follow-up to c2d6278. NPE->ClosedChannelException for closed socket

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new 7046644  Follow-up to c2d6278. NPE->ClosedChannelException for closed socket
7046644 is described below

commit 7046644bf361b89afc246b6643e24ce2ae60cacc
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 29 14:10:29 2019 +0100

    Follow-up to c2d6278. NPE->ClosedChannelException for closed socket
    
    If an attempt is made to use a closed socket, throw a
    ClosedChannelException rather than a NullPointerException
---
 java/org/apache/tomcat/util/net/NioEndpoint.java | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 62dbab2..765935e 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -27,6 +27,7 @@ import java.net.SocketTimeoutException;
 import java.nio.ByteBuffer;
 import java.nio.channels.CancelledKeyException;
 import java.nio.channels.Channel;
+import java.nio.channels.ClosedChannelException;
 import java.nio.channels.CompletionHandler;
 import java.nio.channels.FileChannel;
 import java.nio.channels.NetworkChannel;
@@ -52,6 +53,7 @@ import org.apache.tomcat.util.IntrospectionUtils;
 import org.apache.tomcat.util.collections.SynchronizedQueue;
 import org.apache.tomcat.util.collections.SynchronizedStack;
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
+import org.apache.tomcat.util.net.NioChannel.ClosedNioChannel;
 import org.apache.tomcat.util.net.jsse.JSSESupport;
 
 /**
@@ -1240,6 +1242,10 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
 
         @Override
         protected void doWrite(boolean block, ByteBuffer from) throws IOException {
+            NioChannel socket = getSocket();
+            if (socket instanceof ClosedNioChannel) {
+                throw new ClosedChannelException();
+            }
             if (block) {
                 long writeTimeout = getWriteTimeout();
                 Selector selector = null;
@@ -1249,11 +1255,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
                     // Ignore
                 }
                 try {
-                    pool.write(from, getSocket(), selector, writeTimeout);
+                    pool.write(from, socket, selector, writeTimeout);
                     if (block) {
                         // Make sure we are flushed
                         do {
-                            if (getSocket().flush(true, selector, writeTimeout)) {
+                            if (socket.flush(true, selector, writeTimeout)) {
                                 break;
                             }
                         } while (true);
@@ -1268,7 +1274,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
                 // registered for write once as both container and user code can trigger
                 // write registration.
             } else {
-                if (getSocket().write(from) == -1) {
+                if (socket.write(from) == -1) {
                     throw new EOFException();
                 }
             }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] branch master updated: Follow-up to c2d6278. NPE->ClosedChannelException for closed socket

Posted by Rémy Maucherat <re...@apache.org>.
On Wed, May 29, 2019 at 3:11 PM <ma...@apache.org> wrote:

> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
>      new 7046644  Follow-up to c2d6278. NPE->ClosedChannelException for
> closed socket
> 7046644 is described below
>
> commit 7046644bf361b89afc246b6643e24ce2ae60cacc
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Wed May 29 14:10:29 2019 +0100
>
>     Follow-up to c2d6278. NPE->ClosedChannelException for closed socket
>
>     If an attempt is made to use a closed socket, throw a
>     ClosedChannelException rather than a NullPointerException
>

Good point, this was actually incomplete :( No NPEs occurred with the
testsuite, so I moved on. I had been hunting them down. I'll try to tighten
things up further with additional CLOSED immutable object (and the
ClosedSocketChannel object isn't immutable enough right now ...).

Rémy


> ---
>  java/org/apache/tomcat/util/net/NioEndpoint.java | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
> b/java/org/apache/tomcat/util/net/NioEndpoint.java
> index 62dbab2..765935e 100644
> --- a/java/org/apache/tomcat/util/net/NioEndpoint.java
> +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
> @@ -27,6 +27,7 @@ import java.net.SocketTimeoutException;
>  import java.nio.ByteBuffer;
>  import java.nio.channels.CancelledKeyException;
>  import java.nio.channels.Channel;
> +import java.nio.channels.ClosedChannelException;
>  import java.nio.channels.CompletionHandler;
>  import java.nio.channels.FileChannel;
>  import java.nio.channels.NetworkChannel;
> @@ -52,6 +53,7 @@ import org.apache.tomcat.util.IntrospectionUtils;
>  import org.apache.tomcat.util.collections.SynchronizedQueue;
>  import org.apache.tomcat.util.collections.SynchronizedStack;
>  import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
> +import org.apache.tomcat.util.net.NioChannel.ClosedNioChannel;
>  import org.apache.tomcat.util.net.jsse.JSSESupport;
>
>  /**
> @@ -1240,6 +1242,10 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
>
>          @Override
>          protected void doWrite(boolean block, ByteBuffer from) throws
> IOException {
> +            NioChannel socket = getSocket();
> +            if (socket instanceof ClosedNioChannel) {
> +                throw new ClosedChannelException();
> +            }
>              if (block) {
>                  long writeTimeout = getWriteTimeout();
>                  Selector selector = null;
> @@ -1249,11 +1255,11 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
>                      // Ignore
>                  }
>                  try {
> -                    pool.write(from, getSocket(), selector, writeTimeout);
> +                    pool.write(from, socket, selector, writeTimeout);
>                      if (block) {
>                          // Make sure we are flushed
>                          do {
> -                            if (getSocket().flush(true, selector,
> writeTimeout)) {
> +                            if (socket.flush(true, selector,
> writeTimeout)) {
>                                  break;
>                              }
>                          } while (true);
> @@ -1268,7 +1274,7 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
>                  // registered for write once as both container and user
> code can trigger
>                  // write registration.
>              } else {
> -                if (getSocket().write(from) == -1) {
> +                if (socket.write(from) == -1) {
>                      throw new EOFException();
>                  }
>              }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>