tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1600109 - in /tomcat/trunk/java/org/apache/coyote: ./ ajp/ http11/ spdy/
Date Wed, 04 Jun 2014 11:25:52 GMT
Author: markt
Date: Wed Jun  4 11:25:51 2014
New Revision: 1600109

URL: http://svn.apache.org/r1600109
Log:
Refactoring.
Switch from a boolean to an Enum for error state so we can differentiate between an error that requires the connection is closed after the current response is completed and an error that requires that the connection is closed immediately.
This commit should be a NO-OP. While the different error states are set, the only the presence of an error (or not) is tested - i.e. no change from the implementation prior to this commit.
Try to be consistent when an error occurs. Set the status code first (if required), then set the error state and finally log (if required).

Added:
    tomcat/trunk/java/org/apache/coyote/ErrorState.java   (with props)
Modified:
    tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
    tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
    tomcat/trunk/java/org/apache/coyote/ajp/AjpNio2Processor.java
    tomcat/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java
    tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
    tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java

Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Wed Jun  4 11:25:51 2014
@@ -40,9 +40,9 @@ public abstract class AbstractProcessor<
     protected SocketWrapper<S> socketWrapper = null;
 
     /**
-     * Error flag.
+     * Error state for the request/response currently being processed.
      */
-    protected boolean error;
+    private ErrorState errorState;
 
 
     /**
@@ -69,6 +69,24 @@ public abstract class AbstractProcessor<
 
 
     /**
+     * Update the current error state to the new error state if the new error
+     * state is more severe than the current error state.
+     */
+    protected void setErrorState(ErrorState errorState) {
+        this.errorState = this.errorState.getMostSevere(errorState);
+    }
+
+
+    protected void resetErrorState() {
+        errorState = ErrorState.NONE;
+    }
+
+
+    protected ErrorState getErrorState() {
+        return errorState;
+    }
+
+    /**
      * The endpoint receiving connections that are handled by this processor.
      */
     protected AbstractEndpoint<S> getEndpoint() {

Added: tomcat/trunk/java/org/apache/coyote/ErrorState.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ErrorState.java?rev=1600109&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ErrorState.java (added)
+++ tomcat/trunk/java/org/apache/coyote/ErrorState.java Wed Jun  4 11:25:51 2014
@@ -0,0 +1,69 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.coyote;
+
+public enum ErrorState {
+
+    /**
+     * Not in an error state.
+     */
+    NONE(false, 0, true),
+
+    /**
+     * The current request/response is in an error state and while it is safe to
+     * complete the current response it is not safe to continue to use the
+     * existing connection which must be closed once the response has been
+     * completed.
+     */
+    CLOSE_CLEAN(true, 1, true),
+
+    /**
+     * The current request/response is in an error state and it is not safe to
+     * continue to use the existing connection which must be closed immediately.
+     */
+    CLOSE_NOW(true, 2, false);
+
+    private final boolean error;
+    private final int severity;
+    private final boolean ioAllowed;
+
+    private ErrorState(boolean error, int severity, boolean ioAllowed) {
+        this.error = error;
+        this.severity = severity;
+        this.ioAllowed = ioAllowed;
+    }
+
+    public boolean isError() {
+        return error;
+    }
+
+    /**
+     * Compare this ErrorState with the provided ErrorState and return the most
+     * severe.
+     */
+    public ErrorState getMostSevere(ErrorState input) {
+        if (input.severity > this.severity) {
+            return input;
+        } else {
+            return this;
+        }
+    }
+
+    public boolean isIoAllowed() {
+        return ioAllowed;
+    }
+}

Propchange: tomcat/trunk/java/org/apache/coyote/ErrorState.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java Wed Jun  4 11:25:51 2014
@@ -36,6 +36,7 @@ import org.apache.coyote.AbstractProcess
 import org.apache.coyote.ActionCode;
 import org.apache.coyote.AsyncContextCallback;
 import org.apache.coyote.ByteBufferHolder;
+import org.apache.coyote.ErrorState;
 import org.apache.coyote.InputBuffer;
 import org.apache.coyote.OutputBuffer;
 import org.apache.coyote.Request;
@@ -362,8 +363,7 @@ public abstract class AbstractAjpProcess
             try {
                 finish();
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             break;
         }
@@ -375,15 +375,13 @@ public abstract class AbstractAjpProcess
             try {
                 prepareResponse();
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
 
             try {
                 flush(false);
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             break;
         }
@@ -397,8 +395,7 @@ public abstract class AbstractAjpProcess
                 try {
                     prepareResponse();
                 } catch (IOException e) {
-                    // Set error flag
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_NOW);
                     return;
                 }
             }
@@ -406,19 +403,18 @@ public abstract class AbstractAjpProcess
             try {
                 flush(true);
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             break;
         }
         case IS_ERROR: {
-            ((AtomicBoolean) param).set(error);
+            ((AtomicBoolean) param).set(getErrorState().isError());
             break;
         }
         case DISABLE_SWALLOW_INPUT: {
             // TODO: Do not swallow request input but
             // make sure we are closing the connection
-            error = true;
+            setErrorState(ErrorState.CLOSE_CLEAN);
             break;
         }
         case RESET: {
@@ -691,20 +687,22 @@ public abstract class AbstractAjpProcess
         RequestInfo rp = request.getRequestProcessor();
         try {
             rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
-            error = !getAdapter().asyncDispatch(request, response, status);
+            if(!getAdapter().asyncDispatch(request, response, status)) {
+                setErrorState(ErrorState.CLOSE_NOW);
+            }
             resetTimeouts();
         } catch (InterruptedIOException e) {
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         } catch (Throwable t) {
             ExceptionUtils.handleThrowable(t);
+            setErrorState(ErrorState.CLOSE_NOW);
             getLog().error(sm.getString("http11processor.request.process"), t);
-            error = true;
         }
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
 
         if (isAsync()) {
-            if (error) {
+            if (getErrorState().isError()) {
                 request.updateCounters();
                 return SocketState.CLOSED;
             } else {
@@ -712,7 +710,7 @@ public abstract class AbstractAjpProcess
             }
         } else {
             request.updateCounters();
-            if (error) {
+            if (getErrorState().isError()) {
                 return SocketState.CLOSED;
             } else {
                 return SocketState.OPEN;
@@ -742,11 +740,11 @@ public abstract class AbstractAjpProcess
         boolean cping = false;
 
         // Error flag
-        error = false;
+        resetErrorState();
 
         boolean keptAlive = false;
 
-        while (!error && !endpoint.isPaused()) {
+        while (!getErrorState().isError() && !endpoint.isPaused()) {
             // Parsing the request header
             try {
                 // Get first message of the request
@@ -769,7 +767,7 @@ public abstract class AbstractAjpProcess
                     try {
                         output(pongMessageArray, 0, pongMessageArray.length, true);
                     } catch (IOException e) {
-                        error = true;
+                        setErrorState(ErrorState.CLOSE_NOW);
                     }
                     recycle(false);
                     continue;
@@ -779,24 +777,24 @@ public abstract class AbstractAjpProcess
                     if (getLog().isDebugEnabled()) {
                         getLog().debug("Unexpected message: " + type);
                     }
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_NOW);
                     break;
                 }
                 keptAlive = true;
                 request.setStartTime(System.currentTimeMillis());
             } catch (IOException e) {
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
                 break;
             } catch (Throwable t) {
                 ExceptionUtils.handleThrowable(t);
                 getLog().debug(sm.getString("ajpprocessor.header.error"), t);
                 // 400 - Bad Request
                 response.setStatus(400);
+                setErrorState(ErrorState.CLOSE_CLEAN);
                 getAdapter().log(request, response, 0);
-                error = true;
             }
 
-            if (!error) {
+            if (!getErrorState().isError()) {
                 // Setting up filters, and parse some request headers
                 rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
                 try {
@@ -806,37 +804,37 @@ public abstract class AbstractAjpProcess
                     getLog().debug(sm.getString("ajpprocessor.request.prepare"), t);
                     // 500 - Internal Server Error
                     response.setStatus(500);
+                    setErrorState(ErrorState.CLOSE_CLEAN);
                     getAdapter().log(request, response, 0);
-                    error = true;
                 }
             }
 
-            if (!error && !cping && endpoint.isPaused()) {
+            if (!getErrorState().isError() && !cping && endpoint.isPaused()) {
                 // 503 - Service unavailable
                 response.setStatus(503);
+                setErrorState(ErrorState.CLOSE_CLEAN);
                 getAdapter().log(request, response, 0);
-                error = true;
             }
             cping = false;
 
             // Process the request in the adapter
-            if (!error) {
+            if (!getErrorState().isError()) {
                 try {
                     rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
                     getAdapter().service(request, response);
                 } catch (InterruptedIOException e) {
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_NOW);
                 } catch (Throwable t) {
                     ExceptionUtils.handleThrowable(t);
                     getLog().error(sm.getString("ajpprocessor.request.process"), t);
                     // 500 - Internal Server Error
                     response.setStatus(500);
+                    setErrorState(ErrorState.CLOSE_CLEAN);
                     getAdapter().log(request, response, 0);
-                    error = true;
                 }
             }
 
-            if (isAsync() && !error) {
+            if (isAsync() && !getErrorState().isError()) {
                 break;
             }
 
@@ -846,13 +844,13 @@ public abstract class AbstractAjpProcess
                     finish();
                 } catch (Throwable t) {
                     ExceptionUtils.handleThrowable(t);
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_NOW);
                 }
             }
 
             // If there was an error, make sure the request is counted as
             // and error, and update the statistics counter
-            if (error) {
+            if (getErrorState().isError()) {
                 response.setStatus(500);
             }
             request.updateCounters();
@@ -868,14 +866,14 @@ public abstract class AbstractAjpProcess
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
 
-        if (!error && !endpoint.isPaused()) {
+        if (getErrorState().isError() || endpoint.isPaused()) {
+            return SocketState.CLOSED;
+        } else {
             if (isAsync()) {
                 return SocketState.LONG;
             } else {
                 return SocketState.OPEN;
             }
-        } else {
-            return SocketState.CLOSED;
         }
     }
 
@@ -1193,7 +1191,7 @@ public abstract class AbstractAjpProcess
                 long cl = vMB.getLong();
                 if (contentLengthSet) {
                     response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_CLEAN);
                 } else {
                     contentLengthSet = true;
                     // Set the content-length header for the request
@@ -1308,7 +1306,7 @@ public abstract class AbstractAjpProcess
                     secret = true;
                     if (!tmpMB.equals(requiredSecret)) {
                         response.setStatus(403);
-                        error = true;
+                        setErrorState(ErrorState.CLOSE_CLEAN);
                     }
                 }
                 break;
@@ -1324,7 +1322,7 @@ public abstract class AbstractAjpProcess
         // Check if secret was submitted if required
         if ((requiredSecret != null) && !secret) {
             response.setStatus(403);
-            error = true;
+            setErrorState(ErrorState.CLOSE_CLEAN);
         }
 
         // Check for a full URI (including protocol://host:port/)
@@ -1357,7 +1355,7 @@ public abstract class AbstractAjpProcess
         MessageBytes valueMB = request.getMimeHeaders().getValue("host");
         parseHost(valueMB);
 
-        if (error) {
+        if (getErrorState().isError()) {
             getAdapter().log(request, response, 0);
         }
     }
@@ -1375,7 +1373,7 @@ public abstract class AbstractAjpProcess
                 request.serverName().duplicate(request.localName());
             } catch (IOException e) {
                 response.setStatus(400);
-                error = true;
+                setErrorState(ErrorState.CLOSE_CLEAN);
             }
             return;
         }
@@ -1423,9 +1421,9 @@ public abstract class AbstractAjpProcess
                 int charValue = HexUtils.getDec(valueB[i + valueS]);
                 if (charValue == -1) {
                     // Invalid character
-                    error = true;
                     // 400 - Bad request
                     response.setStatus(400);
+                    setErrorState(ErrorState.CLOSE_CLEAN);
                     break;
                 }
                 port = port + (charValue * mult);
@@ -1544,8 +1542,7 @@ public abstract class AbstractAjpProcess
             try {
                 prepareResponse();
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
         }
 
@@ -1560,7 +1557,7 @@ public abstract class AbstractAjpProcess
         }
 
         // Add the end message
-        if (error) {
+        if (getErrorState().isError()) {
             output(endAndCloseMessageArray, 0, endAndCloseMessageArray.length, true);
         } else {
             output(endMessageArray, 0, endMessageArray.length, true);
@@ -1750,8 +1747,7 @@ public abstract class AbstractAjpProcess
                 try {
                     prepareResponse();
                 } catch (IOException e) {
-                    // Set error flag
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_NOW);
                 }
             }
 

Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpNio2Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpNio2Processor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AjpNio2Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AjpNio2Processor.java Wed Jun  4 11:25:51 2014
@@ -104,7 +104,7 @@ public class AjpNio2Processor extends Ab
         // The NIO connector uses the timeout configured on the wrapper in the
         // poller. Therefore, it needs to be reset once asycn processing has
         // finished.
-        if (!error && socketWrapper != null &&
+        if (!getErrorState().isError() && socketWrapper != null &&
                 asyncStateMachine.isAsyncDispatching()) {
             long soTimeout = endpoint.getSoTimeout();
 

Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java Wed Jun  4 11:25:51 2014
@@ -82,8 +82,9 @@ public class AjpNioProcessor extends Abs
         // The NIO connector uses the timeout configured on the wrapper in the
         // poller. Therefore, it needs to be reset once asycn processing has
         // finished.
-        final NioEndpoint.KeyAttachment attach = (NioEndpoint.KeyAttachment)socketWrapper.getSocket().getAttachment(false);
-        if (!error && attach != null &&
+        final NioEndpoint.KeyAttachment attach =
+                (NioEndpoint.KeyAttachment)socketWrapper.getSocket().getAttachment(false);
+        if (!getErrorState().isError() && attach != null &&
                 asyncStateMachine.isAsyncDispatching()) {
             long soTimeout = endpoint.getSoTimeout();
 

Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Wed Jun  4 11:25:51 2014
@@ -30,6 +30,7 @@ import javax.servlet.http.HttpUpgradeHan
 import org.apache.coyote.AbstractProcessor;
 import org.apache.coyote.ActionCode;
 import org.apache.coyote.AsyncContextCallback;
+import org.apache.coyote.ErrorState;
 import org.apache.coyote.RequestInfo;
 import org.apache.coyote.http11.filters.BufferedInputFilter;
 import org.apache.coyote.http11.filters.ChunkedInputFilter;
@@ -699,7 +700,7 @@ public abstract class AbstractHttp11Proc
             // Unsupported transfer encoding
             // 501 - Unimplemented
             response.setStatus(501);
-            error = true;
+            setErrorState(ErrorState.CLOSE_CLEAN);
             if (getLog().isDebugEnabled()) {
                 getLog().debug(sm.getString("http11processor.request.prepare") +
                           " Unsupported transfer encoding [" + encodingName + "]");
@@ -723,8 +724,7 @@ public abstract class AbstractHttp11Proc
             try {
                 getOutputBuffer().endRequest();
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             break;
         }
@@ -739,8 +739,7 @@ public abstract class AbstractHttp11Proc
                 prepareResponse();
                 getOutputBuffer().commit();
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             break;
         }
@@ -756,8 +755,7 @@ public abstract class AbstractHttp11Proc
             try {
                 getOutputBuffer().sendAck();
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             break;
         }
@@ -765,20 +763,19 @@ public abstract class AbstractHttp11Proc
             try {
                 getOutputBuffer().flush();
             } catch (IOException e) {
-                // Set error flag
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
                 response.setErrorException(e);
             }
             break;
         }
         case IS_ERROR: {
-            ((AtomicBoolean) param).set(error);
+            ((AtomicBoolean) param).set(getErrorState().isError());
             break;
         }
         case DISABLE_SWALLOW_INPUT: {
             // Do not swallow request input and make sure we are closing the
             // connection
-            error = true;
+            setErrorState(ErrorState.CLOSE_CLEAN);
             getInputBuffer().setSwallowInput(false);
             break;
         }
@@ -880,7 +877,7 @@ public abstract class AbstractHttp11Proc
                 isReady.set(getOutputBuffer().isReady());
             } catch (IOException e) {
                 getLog().debug("isReady() failed", e);
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             break;
         }
@@ -965,7 +962,6 @@ public abstract class AbstractHttp11Proc
         getOutputBuffer().init(socketWrapper, endpoint);
 
         // Flags
-        error = false;
         keepAlive = true;
         comet = false;
         openSocket = false;
@@ -976,12 +972,13 @@ public abstract class AbstractHttp11Proc
         } else {
             keptAlive = socketWrapper.isKeptAlive();
         }
+        resetErrorState();
 
         if (disableKeepAlive()) {
             socketWrapper.setKeepAliveLeft(0);
         }
 
-        while (!error && keepAlive && !comet && !isAsync() &&
+        while (!getErrorState().isError() && keepAlive && !comet && !isAsync() &&
                 httpUpgradeHandler == null && !endpoint.isPaused()) {
 
             // Parsing the request header
@@ -997,7 +994,7 @@ public abstract class AbstractHttp11Proc
                 if (endpoint.isPaused()) {
                     // 503 - Service unavailable
                     response.setStatus(503);
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_CLEAN);
                 } else {
                     // Make sure that connectors that are non-blocking during
                     // header processing (NIO) only set the start time the first
@@ -1025,7 +1022,7 @@ public abstract class AbstractHttp11Proc
                     getLog().debug(
                             sm.getString("http11processor.header.parse"), e);
                 }
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
                 break;
             } catch (Throwable t) {
                 ExceptionUtils.handleThrowable(t);
@@ -1047,11 +1044,11 @@ public abstract class AbstractHttp11Proc
                 }
                 // 400 - Bad Request
                 response.setStatus(400);
+                setErrorState(ErrorState.CLOSE_CLEAN);
                 getAdapter().log(request, response, 0);
-                error = true;
             }
 
-            if (!error) {
+            if (!getErrorState().isError()) {
                 // Setting up filters, and parse some request headers
                 rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
                 try {
@@ -1064,8 +1061,8 @@ public abstract class AbstractHttp11Proc
                     }
                     // 500 - Internal Server Error
                     response.setStatus(500);
+                    setErrorState(ErrorState.CLOSE_CLEAN);
                     getAdapter().log(request, response, 0);
-                    error = true;
                 }
             }
 
@@ -1077,7 +1074,7 @@ public abstract class AbstractHttp11Proc
             }
 
             // Process the request in the adapter
-            if (!error) {
+            if (!getErrorState().isError()) {
                 try {
                     rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
                     getAdapter().service(request, response);
@@ -1086,22 +1083,25 @@ public abstract class AbstractHttp11Proc
                     // set the status to 500 and set the errorException.
                     // If we fail here, then the response is likely already
                     // committed, so we can't try and set headers.
-                    if(keepAlive && !error) { // Avoid checking twice.
-                        error = response.getErrorException() != null ||
-                                (!isAsync() &&
-                                statusDropsConnection(response.getStatus()));
+                    if(keepAlive && !getErrorState().isError() && (
+                            response.getErrorException() != null ||
+                                    (!isAsync() &&
+                                    statusDropsConnection(response.getStatus())))) {
+                        setErrorState(ErrorState.CLOSE_CLEAN);
                     }
                     setCometTimeouts(socketWrapper);
                 } catch (InterruptedIOException e) {
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_NOW);
                 } catch (HeadersTooLargeException e) {
-                    error = true;
                     // The response should not have been committed but check it
                     // anyway to be safe
-                    if (!response.isCommitted()) {
+                    if (response.isCommitted()) {
+                        setErrorState(ErrorState.CLOSE_NOW);
+                    } else {
                         response.reset();
                         response.setStatus(500);
-                        response.setHeader("Connection", "close");
+                        setErrorState(ErrorState.CLOSE_CLEAN);
+                        response.setHeader("Connection", "close"); // TODO: Remove
                     }
                 } catch (Throwable t) {
                     ExceptionUtils.handleThrowable(t);
@@ -1109,8 +1109,8 @@ public abstract class AbstractHttp11Proc
                             "http11processor.request.process"), t);
                     // 500 - Internal Server Error
                     response.setStatus(500);
+                    setErrorState(ErrorState.CLOSE_CLEAN);
                     getAdapter().log(request, response, 0);
-                    error = true;
                 }
             }
 
@@ -1118,7 +1118,7 @@ public abstract class AbstractHttp11Proc
             rp.setStage(org.apache.coyote.Constants.STAGE_ENDINPUT);
 
             if (!isAsync() && !comet) {
-                if (error) {
+                if (getErrorState().isError()) {
                     // If we know we are closing the connection, don't drain
                     // input. This way uploading a 100GB file doesn't tie up the
                     // thread if the servlet has rejected it.
@@ -1141,12 +1141,12 @@ public abstract class AbstractHttp11Proc
 
             // If there was an error, make sure the request is counted as
             // and error, and update the statistics counter
-            if (error) {
+            if (getErrorState().isError()) {
                 response.setStatus(500);
             }
             request.updateCounters();
 
-            if (!isAsync() && !comet || error) {
+            if (!isAsync() && !comet || getErrorState().isError()) {
                 getInputBuffer().nextRequest();
                 getOutputBuffer().nextRequest();
             }
@@ -1168,7 +1168,7 @@ public abstract class AbstractHttp11Proc
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
 
-        if (error || endpoint.isPaused()) {
+        if (getErrorState().isError() || endpoint.isPaused()) {
             return SocketState.CLOSED;
         } else if (isAsync() || comet) {
             return SocketState.LONG;
@@ -1223,13 +1223,13 @@ public abstract class AbstractHttp11Proc
         } else {
             // Unsupported protocol
             http11 = false;
-            error = true;
             // Send 505; Unsupported HTTP version
+            response.setStatus(505);
+            setErrorState(ErrorState.CLOSE_CLEAN);
             if (getLog().isDebugEnabled()) {
                 getLog().debug(sm.getString("http11processor.request.prepare")+
                           " Unsupported HTTP version \""+protocolMB+"\"");
             }
-            response.setStatus(505);
         }
 
         MessageBytes methodMB = request.method();
@@ -1262,8 +1262,8 @@ public abstract class AbstractHttp11Proc
                 getInputBuffer().setSwallowInput(false);
                 expectation = true;
             } else {
-                error = true;
                 response.setStatus(HttpServletResponse.SC_EXPECTATION_FAILED);
+                setErrorState(ErrorState.CLOSE_CLEAN);
             }
         }
 
@@ -1355,13 +1355,13 @@ public abstract class AbstractHttp11Proc
 
         // Check host header
         if (http11 && (valueMB == null)) {
-            error = true;
             // 400 - Bad request
+            response.setStatus(400);
+            setErrorState(ErrorState.CLOSE_CLEAN);
             if (getLog().isDebugEnabled()) {
                 getLog().debug(sm.getString("http11processor.request.prepare")+
                           " host header missing");
             }
-            response.setStatus(400);
         }
 
         parseHost(valueMB);
@@ -1375,7 +1375,7 @@ public abstract class AbstractHttp11Proc
             contentDelimitation = true;
         }
 
-        if (error) {
+        if (getErrorState().isError()) {
             getAdapter().log(request, response, 0);
         }
     }
@@ -1526,7 +1526,7 @@ public abstract class AbstractHttp11Proc
                 headers.addValue(Constants.CONNECTION).setString(
                         Constants.CLOSE);
             }
-        } else if (!http11 && !error) {
+        } else if (!http11 && !getErrorState().isError()) {
             headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
         }
 
@@ -1616,9 +1616,9 @@ public abstract class AbstractHttp11Proc
                 int charValue = HexUtils.getDec(valueB[i + valueS]);
                 if (charValue == -1 || charValue > 9) {
                     // Invalid character
-                    error = true;
                     // 400 - Bad request
                     response.setStatus(400);
+                    setErrorState(ErrorState.CLOSE_CLEAN);
                     break;
                 }
                 port = port + (charValue * mult);
@@ -1669,19 +1669,21 @@ public abstract class AbstractHttp11Proc
         RequestInfo rp = request.getRequestProcessor();
         try {
             rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
-            error = !getAdapter().asyncDispatch(request, response, status);
+            if(!getAdapter().asyncDispatch(request, response, status)) {
+                setErrorState(ErrorState.CLOSE_NOW);
+            }
             resetTimeouts();
         } catch (InterruptedIOException e) {
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         } catch (Throwable t) {
             ExceptionUtils.handleThrowable(t);
+            setErrorState(ErrorState.CLOSE_NOW);
             getLog().error(sm.getString("http11processor.request.process"), t);
-            error = true;
         }
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
 
-        if (error) {
+        if (getErrorState().isError()) {
             return SocketState.CLOSED;
         } else if (isAsync()) {
             return SocketState.LONG;
@@ -1744,26 +1746,25 @@ public abstract class AbstractHttp11Proc
         try {
             getInputBuffer().endRequest();
         } catch (IOException e) {
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         } catch (Throwable t) {
             ExceptionUtils.handleThrowable(t);
-            getLog().error(sm.getString("http11processor.request.finish"), t);
             // 500 - Internal Server Error
             // Can't add a 500 to the access log since that has already been
             // written in the Adapter.service method.
             response.setStatus(500);
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
+            getLog().error(sm.getString("http11processor.request.finish"), t);
         }
         try {
             getOutputBuffer().endRequest();
         } catch (IOException e) {
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         } catch (Throwable t) {
             ExceptionUtils.handleThrowable(t);
+            setErrorState(ErrorState.CLOSE_NOW);
             getLog().error(sm.getString("http11processor.response.finish"), t);
-            error = true;
         }
-
     }
 
 
@@ -1793,6 +1794,7 @@ public abstract class AbstractHttp11Proc
         }
         httpUpgradeHandler = null;
         comet = false;
+        resetErrorState();
         recycleInternal();
     }
 

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Wed Jun  4 11:25:51 2014
@@ -23,6 +23,7 @@ import java.security.cert.CertificateFac
 import java.security.cert.X509Certificate;
 
 import org.apache.coyote.ActionCode;
+import org.apache.coyote.ErrorState;
 import org.apache.coyote.RequestInfo;
 import org.apache.coyote.http11.filters.BufferedInputFilter;
 import org.apache.juli.logging.Log;
@@ -111,21 +112,23 @@ public class Http11AprProcessor extends 
 
         try {
             rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
-            error = !getAdapter().event(request, response, status);
+            if (!getAdapter().event(request, response, status)) {
+                setErrorState(ErrorState.CLOSE_NOW);
+            }
         } catch (InterruptedIOException e) {
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         } catch (Throwable t) {
             ExceptionUtils.handleThrowable(t);
-            log.error(sm.getString("http11processor.request.process"), t);
             // 500 - Internal Server Error
             response.setStatus(500);
+            setErrorState(ErrorState.CLOSE_NOW);
             getAdapter().log(request, response, 0);
-            error = true;
+            log.error(sm.getString("http11processor.request.process"), t);
         }
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
 
-        if (error || status==SocketStatus.STOP) {
+        if (getErrorState().isError() || status==SocketStatus.STOP) {
             return SocketState.CLOSED;
         } else if (!comet) {
             inputBuffer.nextRequest();
@@ -175,8 +178,8 @@ public class Http11AprProcessor extends 
         if (endpoint.isPaused()) {
             // 503 - Service unavailable
             response.setStatus(503);
+            setErrorState(ErrorState.CLOSE_CLEAN);
             getAdapter().log(request, response, 0);
-            error = true;
         } else {
             return true;
         }
@@ -200,7 +203,7 @@ public class Http11AprProcessor extends 
     protected boolean breakKeepAliveLoop(SocketWrapper<Long> socketWrapper) {
         openSocket = keepAlive;
         // Do sendfile as needed: add socket to sendfile and end
-        if (sendfileData != null && !error) {
+        if (sendfileData != null && !getErrorState().isError()) {
             sendfileData.socket = socketWrapper.getSocket().longValue();
             sendfileData.keepAlive = keepAlive;
             if (!((AprEndpoint)endpoint).getSendfile().add(sendfileData)) {
@@ -212,7 +215,7 @@ public class Http11AprProcessor extends 
                         log.debug(sm.getString(
                                 "http11processor.sendfile.error"));
                     }
-                    error = true;
+                    setErrorState(ErrorState.CLOSE_NOW);
                 } else {
                     // The sendfile Poller will add the socket to the main
                     // Poller once sendfile processing is complete

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java Wed Jun  4 11:25:51 2014
@@ -24,6 +24,7 @@ import java.net.InetSocketAddress;
 import javax.net.ssl.SSLEngine;
 
 import org.apache.coyote.ActionCode;
+import org.apache.coyote.ErrorState;
 import org.apache.coyote.RequestInfo;
 import org.apache.coyote.http11.filters.BufferedInputFilter;
 import org.apache.juli.logging.Log;
@@ -92,8 +93,10 @@ public class Http11Nio2Processor extends
         RequestInfo rp = request.getRequestProcessor();
         try {
             rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
-            error = !getAdapter().event(request, response, status);
-            if (!error) {
+            if (!getAdapter().event(request, response, status)) {
+                setErrorState(ErrorState.CLOSE_NOW);
+            }
+            if (!getErrorState().isError()) {
                 if (socketWrapper != null) {
                     socketWrapper.setComet(comet);
                     if (comet) {
@@ -114,19 +117,19 @@ public class Http11Nio2Processor extends
                 }
             }
         } catch (InterruptedIOException e) {
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         } catch (Throwable t) {
             ExceptionUtils.handleThrowable(t);
-            log.error(sm.getString("http11processor.request.process"), t);
             // 500 - Internal Server Error
             response.setStatus(500);
+            setErrorState(ErrorState.CLOSE_NOW);
             getAdapter().log(request, response, 0);
-            error = true;
+            log.error(sm.getString("http11processor.request.process"), t);
         }
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
 
-        if (error || status==SocketStatus.STOP) {
+        if (getErrorState().isError() || status==SocketStatus.STOP) {
             return SocketState.CLOSED;
         } else if (!comet) {
             if (keepAlive) {
@@ -172,7 +175,7 @@ public class Http11Nio2Processor extends
 
     @Override
     protected void resetTimeouts() {
-        if (!error && socketWrapper != null &&
+        if (!getErrorState().isError() && socketWrapper != null &&
                 asyncStateMachine.isAsyncDispatching()) {
             long soTimeout = endpoint.getSoTimeout();
 
@@ -236,8 +239,8 @@ public class Http11Nio2Processor extends
         if (endpoint.isPaused()) {
             // 503 - Service unavailable
             response.setStatus(503);
+            setErrorState(ErrorState.CLOSE_CLEAN);
             getAdapter().log(request, response, 0);
-            error = true;
         } else {
             return true;
         }
@@ -271,7 +274,7 @@ public class Http11Nio2Processor extends
             SocketWrapper<Nio2Channel> socketWrapper) {
         openSocket = keepAlive;
         // Do sendfile as needed: add socket to sendfile and end
-        if (sendfileData != null && !error) {
+        if (sendfileData != null && !getErrorState().isError()) {
             ((Nio2Endpoint.Nio2SocketWrapper) socketWrapper).setSendfileData(sendfileData);
             sendfileData.keepAlive = keepAlive;
             if (((Nio2Endpoint) endpoint).processSendfile(
@@ -282,7 +285,7 @@ public class Http11Nio2Processor extends
                 if (log.isDebugEnabled()) {
                     log.debug(sm.getString("http11processor.sendfile.error"));
                 }
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             return true;
         }

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Wed Jun  4 11:25:51 2014
@@ -24,6 +24,7 @@ import java.nio.channels.SelectionKey;
 import javax.net.ssl.SSLEngine;
 
 import org.apache.coyote.ActionCode;
+import org.apache.coyote.ErrorState;
 import org.apache.coyote.RequestInfo;
 import org.apache.coyote.http11.filters.BufferedInputFilter;
 import org.apache.juli.logging.Log;
@@ -93,8 +94,7 @@ public class Http11NioProcessor extends 
      * @throws IOException error during an I/O operation
      */
     @Override
-    public SocketState event(SocketStatus status)
-        throws IOException {
+    public SocketState event(SocketStatus status) throws IOException {
 
         long soTimeout = endpoint.getSoTimeout();
 
@@ -102,8 +102,10 @@ public class Http11NioProcessor extends 
         final NioEndpoint.KeyAttachment attach = (NioEndpoint.KeyAttachment)socketWrapper.getSocket().getAttachment(false);
         try {
             rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
-            error = !getAdapter().event(request, response, status);
-            if ( !error ) {
+            if (!getAdapter().event(request, response, status)) {
+                setErrorState(ErrorState.CLOSE_NOW);
+            }
+            if (!getErrorState().isError()) {
                 if (attach != null) {
                     attach.setComet(comet);
                     if (comet) {
@@ -124,19 +126,19 @@ public class Http11NioProcessor extends 
                 }
             }
         } catch (InterruptedIOException e) {
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         } catch (Throwable t) {
             ExceptionUtils.handleThrowable(t);
-            log.error(sm.getString("http11processor.request.process"), t);
             // 500 - Internal Server Error
             response.setStatus(500);
+            setErrorState(ErrorState.CLOSE_NOW);
+            log.error(sm.getString("http11processor.request.process"), t);
             getAdapter().log(request, response, 0);
-            error = true;
         }
 
         rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
 
-        if (error || status==SocketStatus.STOP) {
+        if (getErrorState().isError() || status==SocketStatus.STOP) {
             return SocketState.CLOSED;
         } else if (!comet) {
             if (keepAlive) {
@@ -170,7 +172,7 @@ public class Http11NioProcessor extends 
     @Override
     protected void resetTimeouts() {
         final NioEndpoint.KeyAttachment attach = (NioEndpoint.KeyAttachment)socketWrapper.getSocket().getAttachment(false);
-        if (!error && attach != null &&
+        if (!getErrorState().isError() && attach != null &&
                 asyncStateMachine.isAsyncDispatching()) {
             long soTimeout = endpoint.getSoTimeout();
 
@@ -234,8 +236,8 @@ public class Http11NioProcessor extends 
         if (endpoint.isPaused()) {
             // 503 - Service unavailable
             response.setStatus(503);
+            setErrorState(ErrorState.CLOSE_CLEAN);
             getAdapter().log(request, response, 0);
-            error = true;
         } else {
             return true;
         }
@@ -271,11 +273,10 @@ public class Http11NioProcessor extends 
 
 
     @Override
-    protected boolean breakKeepAliveLoop(
-            SocketWrapper<NioChannel> socketWrapper) {
+    protected boolean breakKeepAliveLoop(SocketWrapper<NioChannel> socketWrapper) {
         openSocket = keepAlive;
         // Do sendfile as needed: add socket to sendfile and end
-        if (sendfileData != null && !error) {
+        if (sendfileData != null && !getErrorState().isError()) {
             ((KeyAttachment) socketWrapper).setSendfileData(sendfileData);
             sendfileData.keepAlive = keepAlive;
             SelectionKey key = socketWrapper.getSocket().getIOChannel().keyFor(
@@ -289,7 +290,7 @@ public class Http11NioProcessor extends 
                 if (log.isDebugEnabled()) {
                     log.debug(sm.getString("http11processor.sendfile.error"));
                 }
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
             return true;
         }
@@ -297,8 +298,6 @@ public class Http11NioProcessor extends 
     }
 
 
-
-
     @Override
     public void recycleInternal() {
         socketWrapper = null;
@@ -308,7 +307,6 @@ public class Http11NioProcessor extends 
 
     // ----------------------------------------------------- ActionHook Methods
 
-
     /**
      * Send an action to the connector.
      *

Modified: tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java?rev=1600109&r1=1600108&r2=1600109&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java Wed Jun  4 11:25:51 2014
@@ -27,6 +27,7 @@ import javax.servlet.http.HttpUpgradeHan
 import org.apache.coyote.AbstractProcessor;
 import org.apache.coyote.ActionCode;
 import org.apache.coyote.AsyncContextCallback;
+import org.apache.coyote.ErrorState;
 import org.apache.coyote.InputBuffer;
 import org.apache.coyote.OutputBuffer;
 import org.apache.coyote.Request;
@@ -156,15 +157,16 @@ public class SpdyProcessor<S> extends Ab
             rp.setStage(org.apache.coyote.Constants.STAGE_SERVICE);
             getAdapter().service(request, response);
         } catch (InterruptedIOException e) {
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         } catch (Throwable t) {
             ExceptionUtils.handleThrowable(t);
             // log.error(sm.getString("ajpprocessor.request.process"), t);
             // 500 - Internal Server Error
+            // TODO Log this properly
             t.printStackTrace();
             response.setStatus(500);
             getAdapter().log(request, response, 0);
-            error = true;
+            setErrorState(ErrorState.CLOSE_NOW);
         }
 
         // TODO: async, etc ( detached mode - use a special light protocol)
@@ -175,11 +177,11 @@ public class SpdyProcessor<S> extends Ab
                 finish();
             } catch (Throwable t) {
                 ExceptionUtils.handleThrowable(t);
-                error = true;
+                setErrorState(ErrorState.CLOSE_NOW);
             }
         }
 
-        if (error) {
+        if (getErrorState().isError()) {
             response.setStatus(500);
         }
 
@@ -240,13 +242,13 @@ public class SpdyProcessor<S> extends Ab
             break;
         }
         case IS_ERROR: {
-            ((AtomicBoolean) param).set(error);
+            ((AtomicBoolean) param).set(getErrorState().isError());
             break;
         }
         case DISABLE_SWALLOW_INPUT: {
             // TODO: Do not swallow request input but
             // make sure we are closing the connection
-            error = true;
+            setErrorState(ErrorState.CLOSE_CLEAN);
             break;
         }
         case CLOSE: {



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


Mime
View raw message