Closing URLConnection and InputStream correctly?

Question

I've seen many different examples of using HttpURLConnection + InputStream, and closing them (or not closing them) after use. This is what I came up with to make sure everything is closed after finished, whether there's an error or not. Is this valid?:

HttpURLConnection conn = null;
InputStream is = null;
try {
    URL url = new URL("http://example.com");

    // (set connection and read timeouts on the connection)
    conn = (HttpURLConnection)url.openConnection();

    is = new BufferedInputStream(conn.getInputStream());

    doSomethingWithInputStream(is);

} catch (Exception ex) {
} finally {
    if (is != null) {
        try {
            is.close();
        } catch (IOException e) {
        }
    }
    if (conn != null) {
        conn.disconnect();
    }
}

Thanks

score 21 · Accepted Answer · answered Feb 05 '12 at 15:01

21

Yep.. Doing the end part in finally would be best idea because if code fails somewhere, program won't reach till .close(), .disconnect() statements that we keep before catch statements...

If the code fails somewhere and exception is thrown in between of the program, still finally get executed regardless of exception thrown...

answered Feb 05 '12 at 15:01

Fahim Parkar

30,974
45
160
276

8

You also want to setConnectTimeout and setReadTimeout so that in the event that the server is not available or has a read error the connection does not lock up forever. – w.donahue Feb 05 '12 at 15:18
1

@metalideath - definitely, I omitted them just for conciseness, will add a note in the example in case someone uses it as a copy/paste. – user291701 Feb 05 '12 at 15:21
@metalideath : agreed with you... Here I just replied as per question... there are many things in this world, we have to explore it... – Fahim Parkar Feb 05 '12 at 15:25
Just not to ask the same question again I am asking here.Should we do the same if we use outputstram of connection? I guess yes.And also does anything change if we set keepalive to true.. – cacert Sep 17 '13 at 11:15
@cacert : better you have in finally. – Fahim Parkar Sep 17 '13 at 13:45
9

I have one caveat. You should never catch Exception. You should only catch declared Exceptions. In this case, you should catch IOException, since it is the root class of all the exceptions that could be thrown. Here's why: Suppose the `doSomethingWithInputStream()` method has a bug that generates, say, a NullPointerException. You'll never know this because you handle it as if it were an IOException. This will make your code very hard to debug. But if you catch IOException instead of Exception, the NullPointerException will fall through, and you'll see it right away, so you can fix it. – MiguelMunoz Oct 05 '16 at 20:30

score 8 · Answer 2 · answered Oct 23 '20 at 14:15

There is also the new (with Java 7) 'try()' technique

        try (OutputStream os = http.getOutputStream()) {
            os.write(out);
        }

Basically, it will auto-close anything in the try() statement, regardless of whether it is successful or not.

Closing URLConnection and InputStream correctly?

2 Answers2

Linked