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


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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to