You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/06/19 23:15:22 UTC
svn commit: r1137440 - in /tomcat/trunk/java/org/apache/coyote/http11:
Http11AprProcessor.java Http11NioProcessor.java
Author: markt
Date: Sun Jun 19 21:15:22 2011
New Revision: 1137440
URL: http://svn.apache.org/viewvc?rev=1137440&view=rev
Log:
Reset sendfile data in recycle method so prepareRequest can be pulled up
Modified:
tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
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=1137440&r1=1137439&r2=1137440&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Sun Jun 19 21:15:22 2011
@@ -390,7 +390,8 @@ public class Http11AprProcessor extends
@Override
public void recycleInternal() {
- this.socket = null;
+ socket = null;
+ sendfileData = null;
}
@@ -636,7 +637,6 @@ public class Http11AprProcessor extends
http09 = false;
contentDelimitation = false;
expectation = false;
- sendfileData = null;
if (endpoint.isSSLEnabled()) {
request.scheme().setString("https");
}
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=1137440&r1=1137439&r2=1137440&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Sun Jun 19 21:15:22 2011
@@ -447,15 +447,16 @@ public class Http11NioProcessor extends
@Override
public void recycleInternal() {
- this.socket = null;
- this.cometClose = false;
- this.comet = false;
+ socket = null;
+ cometClose = false;
+ comet = false;
remoteAddr = null;
remoteHost = null;
localAddr = null;
localName = null;
remotePort = -1;
localPort = -1;
+ sendfileData = null;
}
@@ -680,7 +681,6 @@ public class Http11NioProcessor extends
http09 = false;
contentDelimitation = false;
expectation = false;
- sendfileData = null;
if (endpoint.isSSLEnabled()) {
request.scheme().setString("https");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1137440 - in /tomcat/trunk/java/org/apache/coyote/http11:
Http11AprProcessor.java Http11NioProcessor.java
Posted by Mark Thomas <ma...@apache.org>.
On 06/07/2011 13:26, Mark Thomas wrote:
> On 06/07/2011 12:26, Konstantin Kolinko wrote:
>> 2011/7/6 Mark Thomas <ma...@apache.org>:
>>> On 06/07/2011 08:55, Konstantin Kolinko wrote:
>>>> I have doubts regarding this change.
>>>>
>>>> If I understand correctly, recycle() (and thus recycleInternal()) is
>>>> not called when several requests are processed in a row.
>>>
>>> Recycle is called between every request, keeping in mind that a Comet
>>> request may involve multiple reads and writes and that Async requests
>>> may have multiple writes.
>>
>> I do not have much time to dig at the moment,
>> but my understanding was that recycle() clears the input buffer. Thus
>> it cannot be called when there is data in it. The method that is
>> called between requests when there are data is prepareRequest().
>>
>> All this happens while looping inside process() call.
>
> I'll double check what happens when pipe-lining requests. That wasn't
> something I had in mind when looking at it before. Worst case, we need
> two recycle methods.
Yep, the sendfileData does need to be nulled. Looking at options for
doing this now.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1137440 - in /tomcat/trunk/java/org/apache/coyote/http11:
Http11AprProcessor.java Http11NioProcessor.java
Posted by Mark Thomas <ma...@apache.org>.
On 06/07/2011 12:26, Konstantin Kolinko wrote:
> 2011/7/6 Mark Thomas <ma...@apache.org>:
>> On 06/07/2011 08:55, Konstantin Kolinko wrote:
>>> I have doubts regarding this change.
>>>
>>> If I understand correctly, recycle() (and thus recycleInternal()) is
>>> not called when several requests are processed in a row.
>>
>> Recycle is called between every request, keeping in mind that a Comet
>> request may involve multiple reads and writes and that Async requests
>> may have multiple writes.
>
> I do not have much time to dig at the moment,
> but my understanding was that recycle() clears the input buffer. Thus
> it cannot be called when there is data in it. The method that is
> called between requests when there are data is prepareRequest().
>
> All this happens while looping inside process() call.
I'll double check what happens when pipe-lining requests. That wasn't
something I had in mind when looking at it before. Worst case, we need
two recycle methods.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1137440 - in /tomcat/trunk/java/org/apache/coyote/http11:
Http11AprProcessor.java Http11NioProcessor.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/7/6 Mark Thomas <ma...@apache.org>:
> On 06/07/2011 08:55, Konstantin Kolinko wrote:
>> I have doubts regarding this change.
>>
>> If I understand correctly, recycle() (and thus recycleInternal()) is
>> not called when several requests are processed in a row.
>
> Recycle is called between every request, keeping in mind that a Comet
> request may involve multiple reads and writes and that Async requests
> may have multiple writes.
I do not have much time to dig at the moment,
but my understanding was that recycle() clears the input buffer. Thus
it cannot be called when there is data in it. The method that is
called between requests when there are data is prepareRequest().
All this happens while looping inside process() call.
>
>> So, in the old code sendfile is set to null before each request, in
>> the new code it is set to null only once after connection is closed.
>
> I don't think so. If you have some analysis that suggests otherwise,
> please share it.
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1137440 - in /tomcat/trunk/java/org/apache/coyote/http11:
Http11AprProcessor.java Http11NioProcessor.java
Posted by Mark Thomas <ma...@apache.org>.
On 06/07/2011 08:55, Konstantin Kolinko wrote:
> I have doubts regarding this change.
>
> If I understand correctly, recycle() (and thus recycleInternal()) is
> not called when several requests are processed in a row.
Recycle is called between every request, keeping in mind that a Comet
request may involve multiple reads and writes and that Async requests
may have multiple writes.
The call is triggered in the Handler implementations in the protocol.
- Http11Protocol: Http11ConnectionHandler#process() calls
processor.recycle() unless the socket is in long poll mode (i.e. the
request is ongoing)
- Http11NioProtocol: Http11ConnectionHandler#process() calls release)
(which in turn calls processor.recycle()) unless the socket is in long
poll mode (i.e. the request is ongoing)
- Http11AprProtocol: Http11ConnectionHandler#process() calls
processor.recycle() unless the socket is in long poll mode (i.e. the
request is ongoing)
The calls to recycle were somewhat randomly spread about the code for
different connectors and there were certainly bugs in the past where a
particular code path could bypass all the recycle calls. One of the
benefits of the recent refactoring is that it is much easier to check
that things are correct and also easier to fix when they are not.
One of the things at the back of my mind when doing the refactoring was
cleaning up the recycle calls. I didn't do it explicitly but it looks
like it has happened anyway as a side-effect of aligning the code. The
only minor issue I can see are some unnecessary calls to recycle in
Http11Processor#process().
> So, in the old code sendfile is set to null before each request, in
> the new code it is set to null only once after connection is closed.
I don't think so. If you have some analysis that suggests otherwise,
please share it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1137440 - in /tomcat/trunk/java/org/apache/coyote/http11:
Http11AprProcessor.java Http11NioProcessor.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
I have doubts regarding this change.
If I understand correctly, recycle() (and thus recycleInternal()) is
not called when several requests are processed in a row.
So, in the old code sendfile is set to null before each request, in
the new code it is set to null only once after connection is closed.
2011/6/20 <ma...@apache.org>:
> Author: markt
> Date: Sun Jun 19 21:15:22 2011
> New Revision: 1137440
>
> URL: http://svn.apache.org/viewvc?rev=1137440&view=rev
> Log:
> Reset sendfile data in recycle method so prepareRequest can be pulled up
>
> Modified:
> tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
> tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
>
> 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=1137440&r1=1137439&r2=1137440&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Sun Jun 19 21:15:22 2011
> @@ -390,7 +390,8 @@ public class Http11AprProcessor extends
>
> @Override
> public void recycleInternal() {
> - this.socket = null;
> + socket = null;
> + sendfileData = null;
> }
>
>
> @@ -636,7 +637,6 @@ public class Http11AprProcessor extends
> http09 = false;
> contentDelimitation = false;
> expectation = false;
> - sendfileData = null;
> if (endpoint.isSSLEnabled()) {
> request.scheme().setString("https");
> }
>
> 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=1137440&r1=1137439&r2=1137440&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Sun Jun 19 21:15:22 2011
> @@ -447,15 +447,16 @@ public class Http11NioProcessor extends
>
> @Override
> public void recycleInternal() {
> - this.socket = null;
> - this.cometClose = false;
> - this.comet = false;
> + socket = null;
> + cometClose = false;
> + comet = false;
> remoteAddr = null;
> remoteHost = null;
> localAddr = null;
> localName = null;
> remotePort = -1;
> localPort = -1;
> + sendfileData = null;
> }
>
>
> @@ -680,7 +681,6 @@ public class Http11NioProcessor extends
> http09 = false;
> contentDelimitation = false;
> expectation = false;
> - sendfileData = null;
> if (endpoint.isSSLEnabled()) {
> request.scheme().setString("https");
> }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org