Correct way to close nested streams and writers in Java

When closing chained streams, you only need to close the outermost stream. Any errors will be propagated up the chain and be caught.

Refer to Java I/O Streams for details.

To address the issue

This isn't right. After you catch and ignore that exception, execution will pick back up after the catch block and the out.close() statement will be executed.

Your colleague makes a good point about the RuntimeException. If you absolutely need the stream to be closed, you can always try to close each one individually, from the outside in, stopping at the first exception.

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 {
    // 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>();

    // 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 final <T extends Closeable> T autoClose(T closeable) {
            closeables_.add(0, closeable);
            return 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
            for (Closeable closeable : closeables_) {
                if (closeable != null) {
                    try {
                        closeable.close();
                    } 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. (Note that the definition of Closeable calls for close() to ignore the call if the stream is already closed)

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 = 
                    autoClose(fileReader = new FileReader("somefile"));
            BufferedReader bufferedReader = 
                    autoClose(bufferedReader = new BufferedReader(fileReader));

            // ... do something with bufferedReader

            // if you need more than one reader or writer
            FileWriter fileWriter = 
                    autoClose(fileWriter = new FileWriter("someOtherFile"));
            BufferedWriter bufferedWriter = 
                    autoClose(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.

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.

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.

Also you dont have to close all nested streams

check this http://ckarthik17.blogspot.com/2011/02/closing-nested-streams.html

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.