Best way to close nested streams in Java?

Question

What is considered the best, most comprehensive way to close nested streams in Java? For example, consider the setup:

FileOutputStream fos = new FileOutputStream(...)
BufferedOS bos = new BufferedOS(fos);
ObjectOutputStream oos = new ObjectOutputStream(bos);

I understand the close operation needs to be insured (probably by using a finally clause). What I wonder about is, is it necessary to explicitly make sure the nested streams are closed, or is it enough to just make sure to close the outer stream (oos)?

One thing I notice, at least dealing with this specific example, is that the inner streams only seem to throw FileNotFoundExceptions. Which would seem to imply that there's not technically a need to worry about closing them if they fail.

Here's what a colleague wrote:

---

Technically, if it were implemented right, closing the outermost stream (oos) should be enough. But the implementation seems flawed.

Example: BufferedOutputStream inherits close() from FilterOutputStream, which defines it as:

 155       public void close() throws IOException {
 156           try {
 157             flush();
 158           } catch (IOException ignored) {
 159           }
 160           out.close();
 161       }

However, if flush() throws a runtime exception for some reason, then out.close() will never be called. So it seems "safest" (but ugly) to mostly worry about closing FOS, which is keeping the file open.

---

What is considered to be the hands-down best, when-you-absolutely-need-to-be-sure, approach to closing nested streams?

And are there any official Java/Sun docs that deal with this in fine detail?

Answer 1

The colleague raises an interesting point, and there are grounds for arguing either way.

Personally, I would ignore the RuntimeException, because an unchecked exception signifies a bug in the program. If the program is incorrect, fix it. You can't "handle" a bad program at runtime.

Answer 2

I usually do the following. First, define a template-method based class to deal with the try/catch mess

import java.io.Closeable;
import java.io.IOException;
import java.util.LinkedList;
import java.util.List;

public abstract class AutoFileCloser {
    private static final Closeable NEW_FILE = new Closeable() {
        public void close() throws IOException {
            // do nothing
        }
    };

    // the core action code that the implementer wants to run
    protected abstract void doWork() throws Throwable;

    // track a list of closeable thingies to close when finished
    private List<Closeable> closeables_ = new LinkedList<Closeable>();

    // mark a new file
    protected void newFile() {
        closeables_.add(0, NEW_FILE);
    }

    // give the implementer a way to track things to close
    // assumes this is called in order for nested closeables,
    // inner-most to outer-most
    protected void watch(Closeable closeable) {
        closeables_.add(0, closeable);
    }

    public AutoFileCloser() {
        // a variable to track a "meaningful" exception, in case
        // a close() throws an exception
        Throwable pending = null;

        try {
            doWork(); // do the real work

        } catch (Throwable throwable) {
            pending = throwable;

        } finally {
            // close the watched streams
            boolean skip = false;
            for (Closeable closeable : closeables_) {
                if (closeable == NEW_FILE) {
                    skip = false;
                } else  if (!skip && closeable != null) {
                    try {
                        closeable.close();
                        // don't try to re-close nested closeables
                        skip = true;
                    } catch (Throwable throwable) {
                        if (pending == null) {
                            pending = throwable;
                        }
                    }
                }
            }

            // if we had a pending exception, rethrow it
            // this is necessary b/c the close can throw an
            // exception, which would remove the pending
            // status of any exception thrown in the try block
            if (pending != null) {
                if (pending instanceof RuntimeException) {
                    throw (RuntimeException) pending;
                } else {
                    throw new RuntimeException(pending);
                }
            }
        }
    }
}

Note the "pending" exception -- this takes care of the case where an exception thrown during close would mask an exception we might really care about.

The finally tries to close from the outside of any decorated stream first, so if you had a BufferedWriter wrapping a FileWriter, we try to close the BuffereredWriter first, and if that fails, still try to close the FileWriter itself.

You can use the above class as follows:

try {
    // ...

    new AutoFileCloser() {
        @Override protected void doWork() throws Throwable {
            // declare variables for the readers and "watch" them
            FileReader fileReader = null;
            BufferedReader bufferedReader = null;
            watch(fileReader = new FileReader("somefile"));
            watch(bufferedReader = new BufferedReader(fileReader));

            // ... do something with bufferedReader

            // if you need more than one reader or writer
            newFile(); // puts a flag in the 
            FileWriter fileWriter = null;
            BufferedWriter bufferedWriter = null;
            watch(fileWriter = new FileWriter("someOtherFile"));
            watch(bufferedWriter = new BufferedWriter(fileWriter));

            // ... do something with bufferedWriter
        }
    };

    // .. other logic, maybe more AutoFileClosers

} catch (RuntimeException e) {
    // report or log the exception
}

Using this approach you never have to worry about the try/catch/finally to deal with closing files again.

If this is too heavy for your use, at least think about following the try/catch and the "pending" variable approach it uses.

Answer 3

This is a surprisingly awkward question. (Even assuming the acquire; try { use; } finally { release; } code is correct.)

If the construction of the decorator fails, then you wont be closing the underlying stream. Therefore you do need to close the underlying stream explicitly, whether in the finally after use or, more diifcult after successfully handing over the resource to the decorator).

If an exception causes execution to fail, do you really want to flush?

Some decorators actually have resources themselves. The current Sun implementation of ZipInputStream for instance has non-Java heap memory allocated.

It has been claimed that (IIRC) two thirds of the resources uses in the Java library are implemented in a clearly incorrect manner.

Whilst BufferedOutputStream closes even on an IOException from flush, BufferedWriter closes correctly.

My advice: Close resources as directly as possible and don't let them taint other code. OTOH, you can spend too much time on this issue - if OutOfMemoryError is thrown it's nice to behave nicely, but other aspects of your program are probably a higher priority and library code is probably broken in this situation anyway. But I'd always write:

final FileOutputStream rawOut = new FileOutputStream(file);
try {
    OutputStream out = new BufferedOutputStream(rawOut);
    ... write stuff out ...
    out.flush();
} finally {
    rawOut.close();
}

(Look: No catch!)

And perhaps use the Execute Around idiom.

Answer 4

Sun's JavaDocs include RuntimeExceptions in their documentation, as shown by InputStream's read(byte[], int, int) method; documented as throwing NullPointerException and IndexOutOfBoundsException.

FilterOutputStream's flush() is only documented as throwing IOException, thus it doesn't actually throw any RuntimeExceptions. Any that could be thrown would most likely be wrapped in an IIOException.

It could still throw an Error, but there's not much you can do about those; Sun recommends that you don't try to catch them.