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;
                     }

---------------
/*
 * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License version 2 only, as
 * published by the Free Software Foundation.
 *
 * This code is distributed in the hope that it will be useful, but WITHOUT
 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
 * version 2 for more details (a copy is included in the LICENSE file that
 * accompanied this code).
 *
* You should have received a copy of the GNU General Public License version
 * 2 along with this work; if not, write to the Free Software Foundation,
 * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
 *
 * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
 * or visit www.oracle.com if you need additional information or have any
 * questions.
 */

/*
 * @test
 * @bug XXXXXX
 * @summary XXXXXXXX
 */

import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpHandler;
import com.sun.net.httpserver.HttpServer;
import java.io.*;
import java.net.*;
import java.util.concurrent.Phaser;

// Racey test, will not always fail, but if it does then we have a problem.

public class InfiniteLoop {

    public static void main(String[] args) throws Exception {
        HttpServer server = HttpServer.create(new InetSocketAddress(0), 0);
        server.createContext("/test/InfiniteLoop", new RespHandler());
        server.start();
        try {
            InetSocketAddress address = server.getAddress();
            URL url = new URL("http://localhost:"; + address.getPort()
                              + "/test/InfiniteLoop");
            final Phaser phaser = new Phaser(2);
            for (int i=0; i<10; i++) {
HttpURLConnection uc = (HttpURLConnection)url.openConnection();
                final InputStream is = uc.getInputStream();
                final Thread thread = new Thread() {
                    public void run() {
                        try {
                            phaser.arriveAndAwaitAdvance();
                            while (is.read() != -1)
                                Thread.sleep(50);
                        } catch (Exception x) { x.printStackTrace(); }
                    }};
                thread.start();
                phaser.arriveAndAwaitAdvance();
                is.close();
                System.out.println("returned from close");
                thread.join();
            }
        } finally {
            server.stop(0);
        }
    }

    static class RespHandler implements HttpHandler {
        static final int RESP_LENGTH = 32 * 1024;
        @Override
        public void handle(HttpExchange t) throws IOException {
            InputStream is  = t.getRequestBody();
            byte[] ba = new byte[8192];
            while(is.read(ba) != -1);

            t.sendResponseHeaders(200, RESP_LENGTH);
            try (OutputStream os = t.getResponseBody()) {
                os.write(new byte[RESP_LENGTH]);
            }
            t.close();
        }
    }
}
---------------

-Chris.



On 12/11/2012 01:21 AM, Martin Buchholz wrote:
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

Reply via email to