4

There are a few examples where people call close() in their finally block when writing to a file, like this:

OutputStream out = null;
try {
    out = new FileOutputStream(file);
    out.write(data);
    out.close();
}
catch (IOException e) {
    Log.e("Exception", "File write failed: " + e.toString());
} finally {
    try {
        if (out != null) {
            out.close();
        }
    } catch (IOException e) {
        Log.e("Exception", "File write failed: " + e.toString());
    }
}

But there are many more examples, including the official Android docs where they don't do that:

try {
    OutputStream out = new FileOutputStream(file);
    out.write(data);
    out.close();
}
catch (IOException e) {
    Log.e("Exception", "File write failed: " + e.toString());
}

Given that the second example is much shorter, is it really necessary to call close() in finally as shown above or is there some mechanism that would clean up the file handle automatically?

Community
  • 1
  • 1
AndreKR
  • 32,613
  • 18
  • 106
  • 168
  • I suspect the close is omitted for brevity in the latter example. – Andy Turner May 04 '16 at 20:49
  • 3
    Those examples are broken ([the comments](http://stackoverflow.com/questions/9292954/how-to-make-a-copy-of-a-file-in-android#comment58700536_9293885) tell it). Unless you want to leak resources, you have to `close` in a `finally` block or with a try-with-resources. – Tunaki May 04 '16 at 20:49
  • 2
    I don't know the consequences of failing to close a file in Android, but the more modern way to insure that a file is closed is to use a try-with-resources statement: https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html – Solomon Slow May 04 '16 at 20:50
  • 1
    Note that the first example is broken because `out` isn't accessible in the scope of the `finally` block. It also doesn't need the first `out.close()`. – Andy Turner May 04 '16 at 20:53
  • @AndreKR it still wouldn't compile - you need to definitely assign `out`. – Andy Turner May 04 '16 at 21:05

4 Answers4

6

Every situation is a little bit different, but it's best to always close in a finally block OR use java's try with resources syntax, which is effectively the same thing.

The risk you run by not closing in the finally block is that you end up with an unclosed stream object after the catch gets triggered. Also, if your stream is buffered, you might need that final close to flush the buffer to ensure that the write is fully complete. It may be a critical error to complete without that final close.

Doug Stevenson
  • 297,357
  • 32
  • 422
  • 441
  • I'm not sure what you mean in your second paragraph. When the `catch` is triggered, something went wrong. In that case the file is most likely not written completely anyway. – AndreKR May 04 '16 at 21:05
  • 1
    Yes, but you're still hanging on to the resource as far as the operating system is concerned. – Louis Wasserman May 04 '16 at 21:10
  • 1
    @AndreKR If you have a block with multiple catches, and one of them is effectively terminating a sequence of writes that are valid (e.g. not an IOException), then a close() in the try block may not flush those writes. A close() in the finally will do that. It's not a common case, but I've seen bugs crop up due to practices like this. Get in the habit of always closing in finally (unless you really know what you're doing), and these problems go away. – Doug Stevenson May 04 '16 at 21:16
  • Oh, multiple `catch`es, I see. – AndreKR May 04 '16 at 23:01
2

Not closing the file may mean the stream contents don't get flushed in a timely way. This is a concern in the case that the code completes normally and doesn't throw an exception.

If you don't close a FileOutputStream, then its finalize method will try to close it. However, finalize doesn't get called immediately, it is not guaranteed to get called at all, see this question. It would be better not to rely on this.

If JDK7's try-with-resources is an option then use it:

try (OutputStream out = new FileOutputStream(file);) {
    out.write(data);
}

Otherwise, for JDK6, make sure the exception thrown on close can't mask any exception thrown in the try block:

OutputStream out = new FileOutputStream(file);
try {
    out.write(data);
} finally {
    try {
        out.close();
    } catch (IOException e) {
        Log.e("Exception", "File write failed: " + e.toString());
    }
}

It may be verbose but it makes sure the file gets closed.

Community
  • 1
  • 1
Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
1

Yes, it is. Unless you let someone else manage that for you.

Some solutions for that:

AndreKR
  • 32,613
  • 18
  • 106
  • 168
0

It is very important it always close when are writing to or reading a file because if you don't close the writer or the reader. You won't be able to write to a file unless you have the reader closed and vice versa.

Also if you don't close, you might cause some memory leaks. Always remember to close whatever you have open. It doesn't hurt to close, if anything it makes your code more efficient.

CodeDon
  • 38
  • 7