Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Java HttpURLConnection InputStream.close() hangs (or works too long?)

First, some background. There is a worker which expands/resolves bunch of short URLS:

http://t.co/example -> http://example.com

So, we just follow redirects. That's it. We don't read any data from the connection. Right after we got 200 we return the final URL and close InputStream.

Now, the problem itself. On a production server one of the resolver threads hangs inside the InputStream.close() call:

"ProcessShortUrlTask" prio=10 tid=0x00007f8810119000 nid=0x402b runnable [0x00007f882b044000]
   java.lang.Thread.State: RUNNABLE
        at java.io.BufferedInputStream.fill(BufferedInputStream.java:218)
        at java.io.BufferedInputStream.skip(BufferedInputStream.java:352)
        - locked <0x0000000561293aa0> (a java.io.BufferedInputStream)
        at sun.net.www.MeteredStream.skip(MeteredStream.java:134)
        - locked <0x0000000561293a70> (a sun.net.www.http.KeepAliveStream)
        at sun.net.www.http.KeepAliveStream.close(KeepAliveStream.java:76)
        at java.io.FilterInputStream.close(FilterInputStream.java:155)
        at sun.net.www.protocol.http.HttpURLConnection$HttpInputStream.close(HttpURLConnection.java:2735)
        at ru.twitter.times.http.URLProcessor.resolve(URLProcessor.java:131)
        at ru.twitter.times.http.URLProcessor.resolve(URLProcessor.java:55)
        at ...

After a brief research, I understood that skip() is called to clean up the stream before sending it back to the connections pool (if keep-alive is set on?). Still I don't understand how to avoid this situation. Moreover, I doubt if there is some bad design in our code or there is problem in JDK.

So, the questions are:

  1. Is it possible to avoid hanging on close()? Guarantee some reasonable timeout, for example.
  2. Is it possible to avoid reading data from connection at all? Remember I just want the final URL. Actually, I think, I don't want skip() to be called at all ...

Update:

KeepAliveStream, line 79, close() method:

    // Skip past the data that's left in the Inputstream because
    // some sort of error may have occurred.
    // Do this ONLY if the skip won't block. The stream may have
    // been closed at the beginning of a big file and we don't want
    // to hang around for nothing. So if we can't skip without blocking
    // we just close the socket and, therefore, terminate the keepAlive
    // NOTE: Don't close super class
    try {
        if (expected > count) {
        long nskip = (long) (expected - count);
        if (nskip <= available()) {
            long n = 0;
            while (n < nskip) {
            nskip = nskip - n;
            n = skip(nskip);} ...

More and more it seems to me that there is a bug in JDK itself. Unfortunately, it's very hard to reproduce this ...

like image 377
Shcheklein Avatar asked Jan 17 '13 12:01

Shcheklein


2 Answers

The implementation of KeepAliveStream that you have linked, violates the contract under which available() and skip() are guaranteed to be non-blocking and thus may indeed block.

The contract of available() guarantees a single non-blocking skip():

Returns an estimate of the number of bytes that can be read (or skipped over) from this input stream without blocking by the next caller of a method for this input stream. The next caller might be the same thread or another thread. A single read or skip of this many bytes will not block, but may read or skip fewer bytes.

Wheres the implementation calls skip() multiple times per single call to available():

    if (nskip <= available()) {
        long n = 0;
        // The loop below can iterate several times,
        // only the first call is guaranteed to be non-blocking. 
        while (n < nskip) { 
        nskip = nskip - n;
        n = skip(nskip);
        }

This doesn't prove that your application blocks because KeepAliveStream incorrectly uses InputStream. Some implementations of InputStream may possibly provide stronger non-blocking guarantees, but I think it is a very likely suspect.

EDIT: After a bit more research, this is a very recently fixed bug in JDK: https://bugs.openjdk.java.net/browse/JDK-8004863?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel. The bug report says about an infinite loop, but a blocking skip() could also be a result. The fix seems to address both issues (there is only a single skip() per available())

like image 68
Jan Wrobel Avatar answered Nov 15 '22 14:11

Jan Wrobel


I guess this skip() on close() is intended for Keep-Alive support.

See http://docs.oracle.com/javase/6/docs/technotes/guides/net/http-keepalive.html.

Prior to Java SE 6, if an application closes a HTTP InputStream when more than a small amount of data remains to be read, then the connection had to be closed, rather than being cached. Now in Java SE 6, the behavior is to read up to 512 Kbytes off the connection in a background thread, thus allowing the connection to be reused. The exact amount of data which may be read is configurable through the http.KeepAlive.remainingData system property.

So keep alive can be effectively disabled with http.KeepAlive.remainingData=0 or http.keepAlive=false. But this can negatively affect performance if you always address to the same http://t.co host.

As @artbristol suggested, using HEAD instead of GET seems to be the preferable solution here.

like image 26
Vadzim Avatar answered Nov 15 '22 14:11

Vadzim