[
https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629996#action_12629996
]
Jason Walton commented on HTTPCORE-172:
---------------------------------------
I agree, actually. I understand exactly where you're coming from. It seems
strange that we would be calling "close()" here, as this is very obviously an
error condition, and you'd mentioned that "shutdown" was for exactly this sort
of thing.
This means Synapse needs to be a bit more diligent, though, in deciding whether
to call close or shutdown. Right now on a "closed" event from the IO Reactor,
Synapse's ClientHandler and ServerHandler always just calls "close()" on both
the SharedInputBuffer and the SharedOutputBuffer (or at least, that's what I
understand Asankha has planned for SYNAPSE-415). The handlers would need to
keep track of their state and figure out which method to call. Perhaps using
"shutdown" here and changing the semantics of shutdown was right, afterall?
Even keeping the semantics of endOfStream the same, though, that doesn't
necessarily mean my fix is a bad one. The only time the exception would be
thrown would be if someone were to call "writeCompleted()" or "close()" and
then try to write more data to the stream from a different thread, which is
obviously wrong.
> 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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
