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

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


The following commit(s) were added to refs/heads/11.0.x by this push:
     new 98363b4d81 Handle NPE observed in unit tests and reported on users 
list.
98363b4d81 is described below

commit 98363b4d818ada51fb6b55e1bcffe366226eded4
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jan 23 16:28:20 2025 +0000

    Handle NPE observed in unit tests and reported on users list.
    
    NPE occurs after an IOException on a non-container thread during
    asynchronous processing.
---
 .../catalina/connector/CoyoteOutputStream.java     | 39 ++++++++++++++++------
 webapps/docs/changelog.xml                         |  5 +++
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/java/org/apache/catalina/connector/CoyoteOutputStream.java 
b/java/org/apache/catalina/connector/CoyoteOutputStream.java
index 9dd3f11197..c093f7d26a 100644
--- a/java/org/apache/catalina/connector/CoyoteOutputStream.java
+++ b/java/org/apache/catalina/connector/CoyoteOutputStream.java
@@ -81,8 +81,7 @@ public class CoyoteOutputStream extends ServletOutputStream {
         try {
             ob.writeByte(i);
         } catch (IOException ioe) {
-            ob.setErrorException(ioe);
-            throw ioe;
+            handleIOException(ioe);
         }
         if (nonBlocking) {
             checkRegisterForWrite();
@@ -102,8 +101,7 @@ public class CoyoteOutputStream extends ServletOutputStream 
{
         try {
             ob.write(b, off, len);
         } catch (IOException ioe) {
-            ob.setErrorException(ioe);
-            throw ioe;
+            handleIOException(ioe);
         }
         if (nonBlocking) {
             checkRegisterForWrite();
@@ -121,8 +119,7 @@ public class CoyoteOutputStream extends ServletOutputStream 
{
         try {
             ob.write(from);
         } catch (IOException ioe) {
-            ob.setErrorException(ioe);
-            throw ioe;
+            handleIOException(ioe);
         }
         if (nonBlocking) {
             checkRegisterForWrite();
@@ -139,8 +136,7 @@ public class CoyoteOutputStream extends ServletOutputStream 
{
         try {
             ob.flush();
         } catch (IOException ioe) {
-            ob.setErrorException(ioe);
-            throw ioe;
+            handleIOException(ioe);
         }
         if (nonBlocking) {
             checkRegisterForWrite();
@@ -179,8 +175,7 @@ public class CoyoteOutputStream extends ServletOutputStream 
{
         try {
             ob.close();
         } catch (IOException ioe) {
-            ob.setErrorException(ioe);
-            throw ioe;
+            handleIOException(ioe);
         }
     }
 
@@ -197,5 +192,29 @@ public class CoyoteOutputStream extends 
ServletOutputStream {
     public void setWriteListener(WriteListener listener) {
         ob.setWriteListener(listener);
     }
+
+
+    private void handleIOException(IOException ioe) throws IOException {
+        try {
+            ob.setErrorException(ioe);
+        } catch (NullPointerException npe) {
+            /*
+             * Ignore.
+             *
+             * An IOException on a non-container thread during asynchronous 
Servlet processing will trigger a dispatch
+             * to a container thread that will complete the asynchronous 
processing and recycle the request, response
+             * and associated objects including the OutputBuffer. Depending on 
timing it is possible that the
+             * OutputBuffer will have been cleared by the time the call above 
is made - resulting in an NPE.
+             *
+             * If the OutputBuffer is null then there is no need to call 
setErrorException(). Catching and ignoring the
+             * NPE is (for now at least) a simpler solution than adding 
locking to OutputBuffer to ensure it is non-null
+             * and remains non-null while setErrorException() is called.
+             *
+             * The longer term solution is likely a refactoring and clean-up 
of error handling for asynchronous requests
+             * but that is potentially a significant piece of work.
+             */
+        }
+        throw ioe;
+    }
 }
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 48a37608ba..4a470c2177 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -176,6 +176,11 @@
         control over the handling of the path used to created a
         <code>RequestDispatcher</code>. (markt)
       </add>
+      <fix>
+        Handle a potential <code>NullPointerException</code> after an
+        <code>IOException</code> occurs on a non-container thread during
+        asynchronous processing. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">


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

Reply via email to