Infinite Loop in KeepAliveStream

Chris Hegarty chris.hegarty at oracle.com
Tue Dec 11 07:40:57 PST 2012


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
>



More information about the net-dev mailing list