[jira] Reopened: (HTTPCORE-172) SharedInputBuffer stops returing data to reading thread once shutdown is called

["Jason Walton (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jason Walton reopened HTTPCORE-172:
-----------------------------------


So close...

With the solution we implemented, there's a bug in SharedOutputBuffer.flushContent():

                while (hasData()) {
                    if (this.shutdown) {
                        throw new InterruptedIOException("Output operation aborted");
                    }
                    this.ioctrl.requestOutput();
                    this.mutex.wait();
                }

That "if(this.shutdown)" needs to have a "|| this.endOfStream" added to it, otherwise if we close a buffer from the I/O Reactor side, then the application may be left trying to write data to it forever (I'm seeing leaked ServerWorkers in Synapse, and this is where they're stuck, so I'm 99% sure it's because of this).

> 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