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 2013/05/10 21:53:21 UTC
svn commit: r1481165 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java InternalNioOutputBuffer.java
Author: markt
Date: Fri May 10 19:53:12 2013
New Revision: 1481165
URL: http://svn.apache.org/r1481165
Log:
Only register for write when using non-blocking and there is more data to write. This fixes various crashes in APR due to trying to add the same socket to the poller twice.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java
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=1481165&r1=1481164&r2=1481165&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Fri May 10 19:53:12 2013
@@ -1057,8 +1057,6 @@ public abstract class AbstractHttp11Proc
rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
- registerForWrite();
-
if (error || endpoint.isPaused()) {
return SocketState.CLOSED;
} else if (isAsync() || comet) {
@@ -1612,7 +1610,6 @@ public abstract class AbstractHttp11Proc
if (error) {
return SocketState.CLOSED;
} else if (isAsync()) {
- registerForWrite();
return SocketState.LONG;
} else {
if (!keepAlive) {
Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java?rev=1481165&r1=1481164&r2=1481165&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java Fri May 10 19:53:12 2013
@@ -153,6 +153,10 @@ public class InternalNioOutputBuffer ext
bytebuffer.clear();
flipped = false;
}
+ if (flipped) {
+ // Still have data to write
+ att.getPoller().add(socket, SelectionKey.OP_WRITE);
+ }
return written;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1481165 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java InternalNioOutputBuffer.java
Posted by Mark Thomas <ma...@apache.org>.
On 11/05/2013 15:56, Konstantin Kolinko wrote:
> 2013/5/10 <ma...@apache.org>:
>> Author: markt
>> Date: Fri May 10 19:53:12 2013
>> New Revision: 1481165
>>
>> URL: http://svn.apache.org/r1481165
>> Log:
>> Only register for write when using non-blocking and there is more data to write. This fixes various crashes in APR due to trying to add the same socket to the poller twice.
>>
>> Modified:
>> tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
>> tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java
>>
>> 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=1481165&r1=1481164&r2=1481165&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
>> +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Fri May 10 19:53:12 2013
>> @@ -1057,8 +1057,6 @@ public abstract class AbstractHttp11Proc
>>
>> rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
>>
>> - registerForWrite();
>> -
>> if (error || endpoint.isPaused()) {
>> return SocketState.CLOSED;
>> } else if (isAsync() || comet) {
>> @@ -1612,7 +1610,6 @@ public abstract class AbstractHttp11Proc
>> if (error) {
>> return SocketState.CLOSED;
>> } else if (isAsync()) {
>> - registerForWrite();
>> return SocketState.LONG;
>> } else {
>> if (!keepAlive) {
>>
>
> (Just noting)
> With this change the "registerForWrite()" method is used in 1 place
> only (AbstractHttp11Processor.java line 1558).
> It can be inlined there and removed from API.
Nice catch. Done.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1481165 - in /tomcat/trunk/java/org/apache/coyote/http11:
AbstractHttp11Processor.java InternalNioOutputBuffer.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/5/10 <ma...@apache.org>:
> Author: markt
> Date: Fri May 10 19:53:12 2013
> New Revision: 1481165
>
> URL: http://svn.apache.org/r1481165
> Log:
> Only register for write when using non-blocking and there is more data to write. This fixes various crashes in APR due to trying to add the same socket to the poller twice.
>
> Modified:
> tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
> tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java
>
> 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=1481165&r1=1481164&r2=1481165&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Fri May 10 19:53:12 2013
> @@ -1057,8 +1057,6 @@ public abstract class AbstractHttp11Proc
>
> rp.setStage(org.apache.coyote.Constants.STAGE_ENDED);
>
> - registerForWrite();
> -
> if (error || endpoint.isPaused()) {
> return SocketState.CLOSED;
> } else if (isAsync() || comet) {
> @@ -1612,7 +1610,6 @@ public abstract class AbstractHttp11Proc
> if (error) {
> return SocketState.CLOSED;
> } else if (isAsync()) {
> - registerForWrite();
> return SocketState.LONG;
> } else {
> if (!keepAlive) {
>
(Just noting)
With this change the "registerForWrite()" method is used in 1 place
only (AbstractHttp11Processor.java line 1558).
It can be inlined there and removed from API.
> Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java?rev=1481165&r1=1481164&r2=1481165&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java Fri May 10 19:53:12 2013
> @@ -153,6 +153,10 @@ public class InternalNioOutputBuffer ext
> bytebuffer.clear();
> flipped = false;
> }
> + if (flipped) {
> + // Still have data to write
> + att.getPoller().add(socket, SelectionKey.OP_WRITE);
> + }
> return written;
> }
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org