On Tue, Dec 11, 2012 at 7:40 AM, Chris Hegarty <chris.hega...@oracle.com>wrote:
> Hi Martin,
>
> Thank you for reporting this issue. I filed 8004863: "Infinite Loop in
> KeepAliveStream", to track it.
>
> I put together a small test to reproduce the problem (inline below). It is
> racey, but shows the problem most of the time on my machine.
>
> I tried your suggested patch, but found that there were cases where not
> enough data was being read off the stream, when it was being closed. This
> causes a problem for subsequent requests on that stream. The suggested
> patch below resolves this, and should also resolve the problem you are
> seeing ( patch against jdk8 ).
>
> Is this something that you want to run with, or would you prefer for me to
> get it into jdk8?
>
> diff -r fda257689786 src/share/classes/sun/net/www/**
> http/KeepAliveStream.java
> --- a/src/share/classes/sun/net/**www/http/KeepAliveStream.java Mon Dec
> 10 10:52:11 2012 +0900
> +++ b/src/share/classes/sun/net/**www/http/KeepAliveStream.java Tue Dec
> 11 15:30:50 2012 +0000
> @@ -83,10 +83,9 @@ class KeepAliveStream extends MeteredStr
> if (expected > count) {
> long nskip = expected - count;
>
> if (nskip <= available()) {
> - long n = 0;
> - while (n < nskip) {
> - nskip = nskip - n;
> - n = skip(nskip);
> + while (nskip > 0) {
> + skip(nskip);
> + nskip = expected - count;
> }
>
I'm still having a hard time understanding the code. It looks to me like
the above still has issues if there is an asynchronous close in this loop.
In that case skip() will make no progress and always return 0, and we will
continue to see nskip > 0.
How about (completely untested)
if (expected > count) {
long nskip = (long) (expected - count);
if (nskip <= available()) {
- long n = 0;
- while (n < nskip) {
- nskip = nskip - n;
- n = skip(nskip);
- }
+ do {} while ((nskip = (long) (expected - count)) > 0L
+ && skip(Math.min(nskip, available())) >
0L);
} else if (expected <=
KeepAliveStreamCleaner.MAX_DATA_REMAINING && !hurried) {
//put this KeepAliveStream on the queue so that the
data remaining
//on the socket can be cleanup asyncronously.
