On 04/06/2014 12:57, Konstantin Kolinko wrote: > 2014-06-04 15:47 GMT+04:00 Mark Thomas <ma...@apache.org>: >> On 04/06/2014 12:36, Konstantin Kolinko wrote: >>> 2014-06-04 15:25 GMT+04:00 <ma...@apache.org>: >>>> 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; >>> >>> You have to assign ErrorState.NONE here by default. >>> Otherwise I expect "setErrorState" to fail with NPE. >> >> Currently not required as resetErrorState() is always called before any >> call to setErrorState(). > > > For HTTP, AJP - OK. I see that resetErrorState() is called in process(). > (I did not notice it at the first glance). > > For AJP : why resetErrorState() is not called in recycle()? (The HTTP > processor calls it).
Because that is what the code did previously. I deliberately opted not to do several refactorings so the commit more obviously introduced no functional changes. >From a consistency point of view, I think that call makes more sense in recycle than in process() which means errorState will need to be initialised. > For SpdyProcessor - broken. It never calls resetErrorState(). Agreed. This will be fixed as a side-effect of the changes above. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
