[
https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12626917#action_12626917
]
Oleg Kalnichevski commented on HTTPCORE-172:
--------------------------------------------
Jason,
The original intent of the #shutdown() method was to shut down the buffer and
notify the consumer about an _abnormal_ termination of the underlying HTTP
connection (due to a timeout or an I/O error). Therefore the content of the
buffer can no longer be considered as being in a consistent state. This is the
reason why read operations terminate with -1 even though there may still be
some data stuck in the internal buffer.
It can well be I have missed valid cases when the SharedInputBuffer may need to
be shut down as a result of a normal sequence of events. I do not mind changing
the semantics of the #shutdown method, but I would like to understand the
rational for that a little better.
A minor comment about the proposed patch. The #hasData() is not threading safe,
so access to it must be synchronized . Therefore isEndOfStream() cannot be
called outside the 'synchronized (this.mutex)' block.
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
>
> 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]
