On 29/05/2014 19:36, Mark Thomas wrote:
> On 29/05/2014 16:27, Konstantin Kolinko wrote:
>> 2014-05-29 18:39 GMT+04:00 <ma...@apache.org>:
>>> Author: markt
>>> Date: Thu May 29 14:39:25 2014
>>> New Revision: 1598307
>>>
>>> URL: http://svn.apache.org/r1598307
>>> Log:
>>> Simplify logic
>>> Call Response.recycle() from Response.reset() rather than in
>>> Response.reset():
>>> - doing ~half of what recycle() does
>>> - calling ActionCode.RESET which resets the OutputBuffer which in turn
>>> calls Response.recycle()
>>
>> There are several places where ActionCode.RESET is used.
>
> I thought I reviewed them but to be honest I have felt like I have been
> going around in circles at times with the connector code so I'll take
> another look.
>
>> Implementation of action(ActionCode.RESET) are different between
>> AbstractHttp11Processor and AbstractAjpProcessor.
>> I can track the above reasoning for HTTP only.
>>
>> I wonder why AJP behaves differently
>
> I half suspect there is no good reason. What has made cleaning up the
> connector code trickier is that implementations started as copy and
> paste and then diverged. That divergence went on for many years in some
> cases. Pulling it back together is taking time.
>
>> and suspect that this change may break it.
>
> My instinct is that it shouldn't. The behaviour of the coyote Response
> object should be independent of the Protocol in use. Equally, based on
> experience, there is a fair chance that something will have broken. Like
> I said above, I'll take another look.
The difference is that for AJP we now call recycle rather than than
doing ~half of what recycle does. The extra calls made are:
commited = false;
commitTime = -1;
These 2 should be NO-OPs as the response has not been committed.
errorException = null;
This one is used to pass back an exception when flush is called so
resetting it here should be fine.
// Servlet 3.1 non-blocking write listener
listener = null;
fireListener = false;
registeredForWrite = false;
I think these are OK as we don't call RESET while we are using async.
That said, if this is an issue we already had the issue for HTTP and at
least now AJP and HTTP will be consistent even if they are wrong (which
I don't think they are).
// update counters
contentWritten=0;
This is a NO-OP as the response has not been committed.
So in summary, I think this change is OK.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
