Closing Streams in Java Source Code ...

Have you seen such code (I see it a lot)?

    private void copyStream(InputStream in, OutputStream out) throws IOException {
        byte[] buf = new byte[1024];
        int len;
        while ((len = in.read(buf)) >= 0) {
            out.write(buf, 0, len);
        }

        out.close();
        in.close();
    }

I don't understand why such a routine code has to be this badly written. This might be one of the reasons Joshua Bloch wants Java 7 to have automatic resource reclamation. But this code is simply bad for following reasons:

  • This method is a function that reads its given input stream and writes it to given output stream. What business does it have closing the streams passed to it????
  • Say there is an exception writing to out. Then, the out.close() and in.close() won't be called and then what should caller do? The caller is simply at the mercy of this code which may or may not close the streams and has no idea what's there in store for it. Wow!

Interestingly, there is no standard detector in FindBugs 1.3.6 that catches this as a bad practice and of course javac does not help at all (with -Xlint:all).

Comments:

http://findbugs.sourceforge.net/bugDescriptions.html#OS_OPEN_STREAM

Fingbugs detects streams which are not closed.

Posted by guest on April 06, 2009 at 11:54 PM PDT #

@193.111.87.20, I know that. But did you actually try it? When I tried:

import java.io.\*;
class BadCode {
private void copyStream(InputStream in, OutputStream out) throws IOException {
byte[] buf = new byte[1024];
int len;
while ((len = in.read(buf)) >= 0) {
out.write(buf, 0, len);
}
out.close();
in.close();
}
}

With FB 1.3.8, I get zero bugs!

Posted by Kedar Mhaswade on April 07, 2009 at 12:15 AM PDT #

I hope you read the description which states if the method creates the stream and then does not close it, it is a bug. In your code you are not creating the streams. If you run findbugs on your entire code, findbugs will point out exactly where you created the stream that it may not be closed in all paths. I am pasting the description below for your convenience.

"The method creates an IO stream object, does not assign it to any fields, pass it to other methods that might close it, or return it, and does not appear to close the stream on all paths out of the method. This may result in a file descriptor leak. It is generally a good idea to use a finally block to ensure that streams are closed."

Posted by 193.111.87.20 on April 07, 2009 at 09:01 PM PDT #

Sorry, anonymous coward at 193.111.87.20,

I did run it on entire source code and Fb 1.38 did come back saying it had zero bugs. I did the sanity check again, with just one class with the same result.

I think you are not understanding the problem with my observation. It either results in leaked file descriptors or redundant close calls on closed streams. Caller has NO idea whatsoever!

Why don't you run Fb yourself and tell me the results?

Posted by Kedar Mhaswade on April 07, 2009 at 10:39 PM PDT #

Actually, I should take this back about Fb. Fb \*does\* tell me that at least one of the streams is not closed, when I call such method (copyStream) and have a finally block that closes the streams naively (os.close(); is.close() in a finally block).

But the point remains, called method had no business closing the streams that it had not opened.

Posted by Kedar Mhaswade on April 08, 2009 at 12:18 AM PDT #

That's why I told you read and learn before "eating humble pie". The method you mentioned is a private method. So it is not part of any public api. So it has no contractual obligation of what it is or it is not supposed to do. I hope you can understand this atleast.

Posted by 193.111.87.20 on April 08, 2009 at 06:09 PM PDT #

Ryan,

I agree. I should have checked. But what threw me off was the private method not opening the streams is closing them. I still maintain that it is a bad practice. You seem to have conveniently punted on that point and are unnecessarily trying to get personal here. I understand your preferences of which blog posts you like to read and comment anonymously (is it?) on, but this kind of response is more wasteful and perhaps harmful.

Posted by Kedar Mhaswade on April 09, 2009 at 03:28 AM PDT #

Post a Comment:
Comments are closed for this entry.
About

Welcome to my blog where mostly my work related thoughts are expressed.

Search

Archives
« April 2014
SunMonTueWedThuFriSat
  
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
   
       
Today