You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Jason Walton (JIRA)" <ji...@apache.org> on 2008/08/28 23:47:44 UTC
[jira] Created: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12626920#action_12626920 ]
Oleg Kalnichevski commented on HTTPCORE-172:
--------------------------------------------
How about adding #close() method for orderly closure of the input buffer?
Oleg
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 690152)
+++ module-nio/src/main/java/org/apache/http/nio/util/SharedInputBuffer.java (working copy)
@@ -120,6 +120,16 @@
}
}
+ public void close() {
+ if (this.shutdown) {
+ return;
+ }
+ this.endOfStream = true;
+ synchronized (this.mutex) {
+ this.mutex.notifyAll();
+ }
+ }
+
protected boolean isShutdown() {
return this.shutdown;
}
> 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ 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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Jason Walton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12626972#action_12626972 ]
Jason Walton commented on HTTPCORE-172:
---------------------------------------
Sounds good. (Actually, when I was looking into SYNAPSE-415, I wondered why the method was called shutdown() instead of close(). :)
We should also add a "close()" method to SharedOutputBuffer for SYNAPSE-415 (since that bug affects both the input and output buffer, and it would be a bit strange to call "close" on one and "shutdown" on the other.)
> 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
RE: [HttpCore] Next release; was Re: [jira] Commented: (HTTPCORE-172)
Posted by "Hubert, Eric" <er...@jamba.net>.
Hi Oleg,
> > just wanted to ask you what are your plans for releasing HttpCore
4.0
> beta3?
> >
> > Do you plan to release it any time soon?
> >
> > Regards,
> > Eric
> I try to make sure there is a release every three months or so. End of
> September or mid October would be the next planned release time frame.
> We can always cut a release sooner if there is enough interest.
We are using Synapse and as Jason I'm very interested in the fixes of
HTTPCORE-170 and HTTPCORE-172 which will go in HttpCore 4.0 beta3. But
as the area of changes is quite limited, I guess the Synapse team will
just patch those classes and update to HttpCore 4.0 beta3 at the moment
it becomes available. If I'm not wrong the Synapse team is going to
release the next beta release end of October so this seems to perfectly
align with your release plans.
Regards,
Eric
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
RE: [HttpCore] Next release; was Re: [jira] Commented: (HTTPCORE-172)
Posted by "Hubert, Eric" <er...@jamba.net>.
Hi Oleg,
> > just wanted to ask you what are your plans for releasing HttpCore
4.0
> beta3?
> >
> > Do you plan to release it any time soon?
> >
> > Regards,
> > Eric
> I try to make sure there is a release every three months or so. End of
> September or mid October would be the next planned release time frame.
> We can always cut a release sooner if there is enough interest.
We are using Synapse and as Jason I'm very interested in the fixes of
HTTPCORE-170 and HTTPCORE-172 which will go in HttpCore 4.0 beta3. But
as the area of changes is quite limited, I guess the Synapse team will
just patch those classes and update to HttpCore 4.0 beta3 at the moment
it becomes available. If I'm not wrong the Synapse team is going to
release the next beta release end of October so this seems to perfectly
align with your release plans.
Regards,
Eric
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
For additional commands, e-mail: dev-help@synapse.apache.org
[HttpCore] Next release; was Re: [jira] Commented: (HTTPCORE-172)
Posted by Oleg Kalnichevski <ol...@apache.org>.
On Sun, 2008-08-31 at 15:30 +0200, Hubert, Eric wrote:
> Oleg,
> just wanted to ask you what are your plans for releasing HttpCore 4.0 beta3?
>
> Do you plan to release it any time soon?
>
> Regards,
> Eric
>
Hi Erik
I try to make sure there is a release every three months or so. End of
September or mid October would be the next planned release time frame.
We can always cut a release sooner if there is enough interest.
Cheers
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
Re: [jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing data to reading thread once shutdown is called
Posted by "Hubert, Eric" <er...@jamba.net>.
Oleg,
just wanted to ask you what are your plans for releasing HttpCore 4.0 beta3?
Do you plan to release it any time soon?
Regards,
Eric
> 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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12627065#action_12627065 ]
Oleg Kalnichevski commented on HTTPCORE-172:
--------------------------------------------
I guess this can be seen either way. I changed the method to not throw IOException.
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
[jira] Updated: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Oleg Kalnichevski updated HTTPCORE-172:
---------------------------------------
Attachment: sharedbuf.patch
How about this patch? Please review and let me know if that solves the issue for you.
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
> 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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629982#action_12629982 ]
Oleg Kalnichevski commented on HTTPCORE-172:
--------------------------------------------
Hi Jason
I see the problem but I am not sure the proposed fix is the right thing to do, as the endOfStream flag should retain its original semantics, that is, indicating that the buffer should close in orderly fashion by flushing buffered content first. I think the right thing to do is to call #shutdown() on the SharedOutputBuffer, not #close(), in case the underlying connection is no longer writable / open.
Does this make sense to you?
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
[jira] Resolved: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Oleg Kalnichevski resolved HTTPCORE-172.
----------------------------------------
Resolution: Fixed
Fix Version/s: 4.0-beta3
Patch checked in. Snapshots deployed.
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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Asankha C. Perera (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12630169#action_12630169 ]
Asankha C. Perera commented on HTTPCORE-172:
--------------------------------------------
Hi Oleg / Jason
Please give me some time to look at this more closely.. I have been working on Synapse 341 and 344, but have found a memory leak which needs urgent fixing.. so I will take a detailed look at the new state of this towards early next week..
thanks
asankha
> 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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629985#action_12629985 ]
Oleg Kalnichevski commented on HTTPCORE-172:
--------------------------------------------
Another alternative would be to have SharedOutputBuffer#close() behave exactly like SharedOutputBuffer#shutdown()
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
[jira] Resolved: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Oleg Kalnichevski resolved HTTPCORE-172.
----------------------------------------
Resolution: Fixed
Asankha,
I committed the fix that I felt should solve the problem. Feel free to re-open the issue at any point of time if you find the solution unsatisfactory.
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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12630013#action_12630013 ]
Oleg Kalnichevski commented on HTTPCORE-172:
--------------------------------------------
This _might_ be a legitimate case, though.I am probably too tired right now to think straight. Let me sleep over it.
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
[jira] Reopened: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Jason Walton (JIRA)" <ji...@apache.org>.
[ 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
[ 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Asankha C. Perera (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12627034#action_12627034 ]
Asankha C. Perera commented on HTTPCORE-172:
--------------------------------------------
Oleg
A minor comment.. I don't see why the close() method would throw an IOException..
thanks
asankha
> 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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Jason Walton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12627002#action_12627002 ]
Jason Walton commented on HTTPCORE-172:
---------------------------------------
This does seem to fix it. Thanks!
> 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
> 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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Jason Walton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12632982#action_12632982 ]
Jason Walton commented on HTTPCORE-172:
---------------------------------------
Looks like the right fix to me, too, Oleg. Thanks again!
> 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
[jira] Commented: (HTTPCORE-172) SharedInputBuffer stops returing
data to reading thread once shutdown is called
Posted by "Jason Walton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HTTPCORE-172?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12629926#action_12629926 ]
Jason Walton commented on HTTPCORE-172:
---------------------------------------
Hmm... And we need a notifyAll() in close.
> 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