tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1600109 - in /tomcat/trunk/java/org/apache/coyote: ./ ajp/ http11/ spdy/
Date Wed, 04 Jun 2014 12:52:00 GMT
On 04/06/2014 12:57, Konstantin Kolinko wrote:
> 2014-06-04 15:47 GMT+04:00 Mark Thomas <markt@apache.org>:
>> On 04/06/2014 12:36, Konstantin Kolinko wrote:
>>> 2014-06-04 15:25 GMT+04:00  <markt@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message