hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Oleg Kalnichevski (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing data to reading thread once shutdown is called
Date Thu, 11 Sep 2008 11:45:44 GMT

    [ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12630168#action_12630168
] 

Oleg Kalnichevski commented on HTTPCORE-172:
--------------------------------------------

Jason,

The shared input buffer can still be in a valid state even if the underlying connection has
been terminated, as long as it contains buffered data from previous reads. The shared output
buffer is different. It is no longer in a valid state if the underlying connection has been
terminated regardless of the buffered data. It is either open (usable) or it is not. So, I
think the simplest and cleanest fix for this problem is to have #close() method simply call
#shutdown(). The consumer would still be expected to call #shutdown() on associated shared
buffers in case of an abnormal termination of the connection and to call #close() in case
of a normal one. It just would not make a difference for the output buffer.

Would this approach fix the problem for Synapse?

Oleg

> SharedInputBuffer stops returing data to reading thread once shutdown is called
> -------------------------------------------------------------------------------
>
>                 Key: HTTPCORE-172
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-172
>             Project: HttpComponents HttpCore
>          Issue Type: Bug
>          Components: HttpCore NIO
>    Affects Versions: 4.0-beta2
>         Environment: Synapse 1.2
>            Reporter: Jason Walton
>             Fix For: 4.0-beta3
>
>         Attachments: sharedbuf.patch
>
>
> This problem won't happen in Synapse 1.2 without the fix I proposed for https://issues.apache.org/jira/browse/SYNAPSE-415
in ClientHandler.java.
> To briefly recap:  If a ClientWorker attempts to read from a SharedInputBuffer, but no
data is availabled, SharedInputBuffer.read() will call into waitForData().  If the next event
to the ClientHandler is a "closed()" event, then the ClientHandler will remove its references
to the SharedInputBuffer.  At this point in time, the ClientWorker is left waiting for data
forever.  The fix I proposed for SYNAPSE-415 was to call into SharedInputBuffer.shutdown()
from ClientHandler.closed(), as this will send a notify to the SharedInputBuffer and wake
up the ClientWorker.
> This seemed like a good fix at the time, but now I'm running into this from the "other
side", so to speak.  Suppose the ClientWorker is a bit late in reading the stream.  
> 1) I/O Dispatcher thread receives data from the socket, and calls ClientHandler.inputReady(),
which in turn calls SharedInputBuffer.consumeContent().  This puts some data into the SharedInputBuffer's
buffer.
> 2) I/O Dispathcer thread sees the socket close, and so calls ClientHandler.closed().
 This calls into SharedInputBuffer.shutdown().
> 3) The ClientWorker thread calls into SharedInputBuffer.read(), which starts out with:
>         if (this.shutdown) {
>             return -1;
>         }
> The result is that the ClientWorker will erroneously think we never got a reply.
> A similar issue exists in SharedOutputBuffer, except here you would have to call "shutdown()"
followed by "produceContent()".  Since both of these methods are called from the I/O dispatcher
side, this wouldn't make a lot of sense (how can we produce content for a socket which is
closed?).  I suppose someone might call "shutdown()" instead of "writeCompleted()" in some
sort of error scenario on the worker side?  Probably not really worth worrying about.
> Here's my proposed fix (comments welcome, since I don't know this code as well as I could):
> Index: module-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java
> ===================================================================
> --- module-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java   (revision
689999)
> +++ module-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java   (working
copy)
> @@ -125,11 +125,11 @@
>      }
>      protected boolean isEndOfStream() {
> -        return this.shutdown || (!hasData() && this.endOfStream);
> +        return !hasData() && (this.shutdown || this.endOfStream);
>      }
>      public int read() throws IOException {
> -        if (this.shutdown) {
> +        if (this.isEndOfStream()) {
>              return -1;
>          }
>          synchronized (this.mutex) {
> @@ -144,7 +144,7 @@
>      }
>      public int read(final byte[] b, int off, int len) throws IOException {
> -        if (this.shutdown) {
> +        if (this.isEndOfStream()) {
>              return -1;
>          }
>          if (b == null) {
> @@ -168,7 +168,7 @@
>      }
>      public int read(final byte[] b) throws IOException {
> -        if (this.shutdown) {
> +        if (this.isEndOfStream()) {
>              return -1;
>          }
>          if (b == null) {

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message