tags:

views:

305

answers:

3

I've see this sort of thing in Java code quite often...

try
{
    fileStream.close();
}
catch (IOException ioe)
{
    /* Ignore. We do not care. */
}

Is this reasonable, or cavalier?

When would I care that closing a file failed? What are the implications of ignoring this exception?

+19  A: 

You would care if the close() method flushes written content from a buffer to the filesystem, and that fails. e.g. if the file you're writing to is on a remote filesystem that has become unavailable.

Note that the above re. flushing applies to any output stream, not just files.

Brian Agnew
+20  A: 

I would at the very least log the exception.

I've seen it happen occasionally, if the attempt to close the file fails due to it not being able to flush the data. If you just swallow the exception, then you've lost data without realizing it.

Ideally, you should probably swallow the exception if you're already in the context of another exception (i.e. you're in a finally block, but due to another exception rather than having completed the try block) but throw it if your operation is otherwise successful. Unfortunately that's somewhat ugly to sort out :(

But yes, you should at least log it.

Jon Skeet
File i/o in Java is a pain. Not the i/o itself, but properly handling closing, exceptions, and so forth cleanly and elegantly. You end up with a huge mess of nested try/catch/finally blocks.
Adam Jaskiewicz
+4  A: 

The most common close() problems are out-of-disk space or, as Brian mentioned, a remote stream that's gone poof.

NOTE:

You should really see something like (note: I haven't compile-checked this)

SomeKindOfStream stream = null;
Throwable pending = null;
try {
    stream = ...;
    // do stuff with stream

} catch (ThreadDeath t) {
    // always re-throw thread death immediately
    throw t;

} catch (Throwable t) {
    // keep track of any exception - we don't want an exception on
    //   close() to hide the exceptions we care about!
    pending = t;

} finally {
    if (stream != null)
        try {
            stream.close();
        } catch (IOException e) {
            if (pending == null)
                pending = e;
        }
    }
    if (pending != null) {
        // possibly log - might log in a caller
        throw new SomeWrapperException(pending);
          // where SomeWrapperException is unchecked or declared thrown
    }
}

Why all this?

Keep in mind that Java can only track one "pending" exception at a time. If the body of the main try block throws an exception, and the close() in the finally throws an exception, the only thing you'll know about is the close().

The above structure does the following:

  • Keep track of any throwable thrown in the body of the try
  • In case that exception is thread death, rethrow it immediately!
  • When closing, if we don't have a pending exception, track the close exception; otherwise the previously-thrown exception should be kept track of. (You should probably try to log the close() error in this case)
  • At the end, if there is a pending exception, deal with it. I usually wrap it and rethrow it. Personally, I use an unchecked wrapper so I don't need to have all callers in the call chain declare throws.

To do the above, I usually use the template method pattern to create the exception management and then override a doWork() method that is the body of the try.

Scott Stanchfield