Infinite Loop in KeepAliveStream

[Martin Buchholz]

Hi sun/net/www maintainers,

Here at Google we have observed an infinite loop
in jdk/src/share/classes/sun/net/www/http/KeepAliveStream.java

 85:                 if (nskip <= available()) {
 86:                     long n = 0;
 87:                     while (n < nskip) {
 88:                         nskip = nskip - n;
 89:                         n = skip(nskip);
 90:                     }

If this stream is asynchronously closed (or perhaps, read, or skipped),
then skip will always return 0 (see MeteredStream)

    public synchronized long skip(long n) throws IOException {

        // REMIND: what does skip do on EOF????
        if (closed) {
            return 0;
        }

and you can clearly see an infinite loop.

Unfortunately, we are too clueless about this code to be able to reproduce
this infloop at will, but we are guessing you may be able to.

Here's an infloop stack trace snippet (line numbers from openjdk6 sources)

 sun.net.www.http.KeepAliveStream.close(KeepAliveStream.java:93)
 java.io.FilterInputStream.close(FilterInputStream.java:172)
 
sun.net.www.protocol.http.HttpURLConnection$HttpInputStream.close(HttpURLConnection.java:2625)
 org.apache.xerces.impl.XMLEntityManager$RewindableInputStream.close(Unknown
Source)
 org.apache.xerces.impl.io.UTF8Reader.close(Unknown Source)
 org.apache.xerces.impl.XMLEntityManager.closeReaders(Unknown Source)
 org.apache.xerces.parsers.XML11Configuration.cleanup(Unknown Source)
 org.apache.xerces.parsers.XML11Configuration.parse(Unknown Source)

Here's a possible patch that seems to make the code more correct in the
presence of asynchronous close, although guaranteeing completely correct
synchronization here seems difficult.

@@ -83,10 +83,8 @@
             if (expected > count) {
                 long nskip = (long) (expected - count);
                 if (nskip <= available()) {
-                    long n = 0;
-                    while (n < nskip) {
-                        nskip = nskip - n;
-                        n = skip(nskip);
+                    while (nskip > 0 && nskip <= available()) {
+                        nskip -= skip(nskip);
                     }
                 } else if (expected <=
KeepAliveStreamCleaner.MAX_DATA_REMAINING && !hurried) {
                     //put this KeepAliveStream on the queue so that the
data remaining

It's very likely you can come up with a better patch.

see also http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4392195