Java try/catch/finally best practices while acquiring/closing resources

Austin Hyde picture Austin Hyde · Nov 4, 2010 · Viewed 55.7k times · Source

While working on a school project, I wrote the following code:

FileOutputStream fos;
ObjectOutputStream oos;
try {
    fos = new FileOutputStream(file);
    oos = new ObjectOutputStream(fos);

    oos.writeObject(shapes);
} catch (FileNotFoundException ex) {
    // complain to user
} catch (IOException ex) {
    // notify user
} finally {
    if (oos != null) oos.close();
    if (fos != null) fos.close();
}

The problem is that Netbeans is telling me the resource.close() lines throw an IOException and therefore must either be caught or declared. It also is complaining that oos and fos might not yet be initialized (despite the null checks).

This seems a little strange, seeing as how the whole point is to stop the IOException right there.

My knee-jerk fix is to do this:

} finally {
    try {
        if (oos != null) oos.close();
        if (fos != null) fos.close();
    } catch (IOException ex) { }
}

But deep down this bothers me and feels dirty.

I come from a C# background, where I would simply take advantage of a using block, so I am unsure of what the "right" way is to handle this.

What is the right way to handle this problem?

Answer

Stephen C picture Stephen C · Nov 4, 2010

If you are trying to catch and report all exceptions at source, a better solution is this:

ObjectOutputStream oos = null;
try {
   oos = new ObjectOutputStream(new FileOutputStream(file));
   oos.writeObject(shapes);
   oos.flush();
} catch (FileNotFoundException ex) {
    // complain to user
} catch (IOException ex) {
    // notify user
} finally {
    if (oos != null) {
        try {
            oos.close();
        } catch (IOException ex) {
            // ignore ... any significant errors should already have been
            // reported via an IOException from the final flush.
        }
    }
}

Notes:

  • The standard Java wrapper streams, readers and writers all propagate close and flush to their wrapped streams, etc. So you only need to close or flush the outermost wrapper.
  • The purpose of flushing explicitly at the end of the try block is so that the (real) handler for IOException gets to see any write failures1.
  • When you do a close or flush on an output stream, there is a "once in a blue moon" chance that an exception will be thrown due to disc errors or file system full. You should not squash this exception!.

If you often have to "close a possibly null stream ignoring IOExceptions", then you could write yourself a helper method like this:

public void closeQuietly(Closeable closeable) {
    if (closeable != null) {
        try {
            closeable.close();
        } catch (IOException ex) {
            // ignore
        }
    }
}

then you can replace the previous finally block with:

} finally {
    closeQuietly(oos);
}

(Another answer points out that a closeQuietly method is already available in an Apache Commons library ... if you don't mind adding a dependency to your project for a 10 line method. UPDATE : note that these methods are deprecated in version 2.6 of the API.)

But be careful that you only use closeQuietly on streams where IO exceptions really are irrelevant.

1 - That is not necessary when using try-with-resources.


On the issue of flush() versus close() that people are asking about:

  • The standard "filter" and "buffered" output streams and writers have an API contract that states that close() causes all buffered output to be flushed. You should find that all other (standard) output classes that do output buffering will behave the same way. So, for a standard class it is redundant to call flush() immediately before close().
  • For custom and 3rd-party classes, you need to investigate (e.g. read the javadoc, look at the code), but any close() method that doesn't flush buffered data is arguably broken.
  • Finally, there is the issue of what flush() actually does. What the javadoc says is this (for OutputStream ...)

    If the intended destination of this stream is an abstraction provided by the underlying operating system, for example a file, then flushing the stream guarantees only that bytes previously written to the stream are passed to the operating system for writing; it does not guarantee that they are actually written to a physical device such as a disk drive.

    So ... if you hope / imagine that calling flush() guarantees that your data will persist, you are wrong! (If you need to do that kind of thing, look at the FileChannel.force method ...)


On the other hand, if you can use Java 7 or later, the "new" try-with-resources as described in @Mike Clark's answer is the best solution.

If you are not using Java 7 or later for your new code, you are probably in a deep hole and digging deeper.