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

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


The following commit(s) were added to refs/heads/9.0.x by this push:
     new a84cdbf  Wrap 'error' and 'applicationIOE' with AtomicReference
a84cdbf is described below

commit a84cdbfcdabefef4ed0030f430eecf7435a8ef58
Author: Martin Tzvetanov Grigorov <mgrigo...@apache.org>
AuthorDate: Thu Oct 1 13:29:42 2020 +0300

    Wrap 'error' and 'applicationIOE' with AtomicReference
    
    Under high load it is possible that one thread makes the check for non-null 
and before the copy another thread to null-fy the member field.
    
    SEVERE: Servlet.service() for servlet [plaintext] in context with path [] 
threw exception
    java.lang.NullPointerException: Cannot throw exception because "ioe" is null
        at 
org.apache.coyote.http2.Http2UpgradeHandler.handleAppInitiatedIOException(Http2UpgradeHandler.java:797)
        at 
org.apache.coyote.http2.Http2AsyncUpgradeHandler.handleAsyncException(Http2AsyncUpgradeHandler.java:276)
        at 
org.apache.coyote.http2.Http2AsyncUpgradeHandler.writeWindowUpdate(Http2AsyncUpgradeHandler.java:252)
        at 
org.apache.coyote.http2.Stream$StreamInputBuffer.doRead(Stream.java:1088)
        at org.apache.coyote.Request.doRead(Request.java:555)
        at 
org.apache.catalina.connector.InputBuffer.realReadBytes(InputBuffer.java:336)
        at 
org.apache.catalina.connector.InputBuffer.checkByteBufferEof(InputBuffer.java:632)
        at org.apache.catalina.connector.InputBuffer.read(InputBuffer.java:362)
        at 
org.apache.catalina.connector.CoyoteInputStream.read(CoyoteInputStream.java:132)
        at org.apache.catalina.connector.Request.readPostBody(Request.java:3308)
        at 
org.apache.catalina.connector.Request.parseParameters(Request.java:3241)
        at org.apache.catalina.connector.Request.getParameter(Request.java:1124)
        at 
org.apache.catalina.connector.RequestFacade.getParameter(RequestFacade.java:381)
        at 
info.mgsolutions.tomcat.PlainTextServlet.doPost(PlainTextServlet.java:41)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:652)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:733)
        at 
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
      ...
    
    (cherry picked from commit 71b70117042bf646183e9e0395625dc1dcbf1333)
---
 .../coyote/http2/Http2AsyncUpgradeHandler.java     | 36 ++++++++++++----------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java 
b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
index eb116a7..2068044 100644
--- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
@@ -25,6 +25,7 @@ import java.nio.file.StandardOpenOption;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 
 import javax.servlet.http.WebConnection;
 
@@ -43,8 +44,8 @@ public class Http2AsyncUpgradeHandler extends 
Http2UpgradeHandler {
     // Because of the compression used, headers need to be written to the
     // network in the same order they are generated.
     private final Object headerWriteLock = new Object();
-    private Throwable error = null;
-    private IOException applicationIOE = null;
+    private final AtomicReference<Throwable> error = new AtomicReference<>();
+    private final AtomicReference<IOException> applicationIOE = new 
AtomicReference<>();
 
     public Http2AsyncUpgradeHandler(Http2Protocol protocol, Adapter adapter,
             Request coyoteRequest) {
@@ -57,7 +58,7 @@ public class Http2AsyncUpgradeHandler extends 
Http2UpgradeHandler {
         }
         @Override
         public void failed(Throwable t, Void attachment) {
-            error = t;
+            error.set(t);
         }
     };
     private final CompletionHandler<Long, Void> applicationErrorCompletion = 
new CompletionHandler<Long, Void>() {
@@ -67,9 +68,9 @@ public class Http2AsyncUpgradeHandler extends 
Http2UpgradeHandler {
         @Override
         public void failed(Throwable t, Void attachment) {
             if (t instanceof IOException) {
-                applicationIOE = (IOException) t;
+                applicationIOE.set((IOException) t);
             }
-            error = t;
+            error.set(t);
         }
     };
 
@@ -109,12 +110,13 @@ public class Http2AsyncUpgradeHandler extends 
Http2UpgradeHandler {
                 TimeUnit.MILLISECONDS, null, SocketWrapperBase.COMPLETE_WRITE, 
errorCompletion,
                 ByteBuffer.wrap(localSettings.getSettingsFrameForPending()),
                 ByteBuffer.wrap(createWindowUpdateForSettings()));
-        if (error != null) {
+        Throwable err = error.get();
+        if (err != null) {
             String msg = sm.getString("upgradeHandler.sendPrefaceFail", 
connectionId);
             if (log.isDebugEnabled()) {
                 log.debug(msg);
             }
-            throw new ProtocolException(msg, error);
+            throw new ProtocolException(msg, err);
         }
     }
 
@@ -270,17 +272,17 @@ public class Http2AsyncUpgradeHandler extends 
Http2UpgradeHandler {
 
 
     private void handleAsyncException() throws IOException {
-        if (applicationIOE != null) {
-            IOException ioe = applicationIOE;
-            applicationIOE = null;
+        IOException ioe = applicationIOE.getAndSet(null);
+        if (ioe != null) {
             handleAppInitiatedIOException(ioe);
-        } else if (error != null) {
-            Throwable error = this.error;
-            this.error = null;
-            if (error instanceof IOException) {
-                throw (IOException) error;
-            } else {
-                throw new IOException(error);
+        } else {
+            Throwable err = this.error.getAndSet(null);
+            if (err != null) {
+                if (err instanceof IOException) {
+                    throw (IOException) err;
+                } else {
+                    throw new IOException(err);
+                }
             }
         }
     }


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

Reply via email to