Correct way to close nested streams and writers in Java [duplicate]

Question

Note: This question and most of its answers date to before the release of Java 7. Java 7 provides Automatic Resource Management functionality for doing this easilly. If you are using Java 7 or later you should advance to the answer of Ross Johnson.

---

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?

Pizor

@BalusC, why a question asked in 2009 is marked as a duplicate of a question asked in 2015? – Dekow

Answer 1

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.

Kitchener

Could you not also write e.g. FileReader fileReader = autoClose(new FileReader("somefile")); and have the same effect? – Isotope

Avoid FileReader/Writer- for one no character encoding – Distribution

@Mr_and_Mrs_D: Off topic; the Q&A here is about closing nested streams. FileReader/Writer does encode - it uses the current platform's default character encoding, which often can be fine. If you need to specify encodings (for transferring files to other systems or such), you can use InputStreamReader/Writer with an encoding option. – Kitchener

yes off topic I know but since this is the accepted answer in a highly viewed question I just wanted to note this - still the default encoding is seldom a good idea :) – Distribution

I'm really not a fan of this. This is really ugly. You've just lost all checked exceptions. Callers of a method that use AutoCloser now have no idea what they should be reasonably expected to handle and recover from. Also you've also taken Error's and made them into RuntimeException's. – Rikki

@Steiny Many people (like me) feel checked exceptions are evil... Think about what happens when you add file IO to a method that's already called from other methods; you would need to modify the signature of all calling methods if you want to be able to catch the checked exception (and 99% of the time, all you care about is "something bad happened" and cannot do any meaningful recovery anyway). The only way to make this generic before Java 7 is to lump all exceptions together. – Kitchener

Answer 2

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

However, if flush() throws a runtime exception for some reason, then out.close() will never be called.

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.

Lawrencelawrencium

If I'm reading it right, his colleague is advocating closing everything in order to handle a RuntimeException which will not be caught in the catch block. Are you agreeing or disagreeing with that? – Improvvisatore

His concern was that out.close() wouldn't be called, which is wrong. It will be called regardless of the ignored exception. – Lawrencelawrencium

If flush throws a runtime exception, close is not called. – Collinsia

^ But it won't be called if flush throws a RuntimeException, correct? – Palmitate

Ah, you guys are right. Anything higher than IOException won't be caught. – Lawrencelawrencium

close methods in java.io have been buggy: bugs.sun.com/bugdatabase/view_bug.do?bug_id=6266377 – Wisecrack

I wouldn't stop, I'd keep closing and log the exception. The case with the BufferedStream in poignant. You (likely) don't want the FileOutputStream lingering around "unclosed" if the BufferedStream implodes during the close. – Dwt

Exactly what kind of RunTime errors do you expect flush to throw while doing a disk write? – Grimsby

Perhaps I should rephrase that: Suns JavaDocs include runtime exceptions in their method declarations, as shown by InputStream's read method being documented as throwing NullPointerException and IndexOutOfBoundsException for the versions with arguments those apply to. OutputStream's flush() is only documented as throwing IOException. – Grimsby

@Will: Good point about continuing and logging. – Lawrencelawrencium

@R. Bemrose: A method is not required to declare in its throws clause any subclasses of RuntimeException that might be thrown during the execution of the method but not caught. java.sun.com/j2se/1.4.2/docs/api/java/lang/… – Lawrencelawrencium

@Bill the Lizard: And.... that brings us back to my first question: Exacttly what kind of Runtime Exception is flush() going to throw? – Grimsby

@R. Bemrose: I don't know what undeclared exceptions flush might call. – Lawrencelawrencium

Answer 3

In the Java 7 era, try-with-resources is certainly the way to go. As mentioned in several previous answers, the close request propagates from the outermost stream to the innermost stream. So a single close is all that is required.

try (ObjectInputStream ois = new ObjectInputStream(new FileInputStream(f))) {
  // do something with ois
}

There is however a problem with this pattern. The try-with-resources is not aware of the inner FileInputStream, so if the ObjectInputStream constructor throws an exception, the FileInputStream is never closed (until the garbage collector gets to it). The solution is...

try (FileInputStream fis = new FileInputStream(f); ObjectInputStream ois = new ObjectInputStream(fis)) {
  // do something with ois
}

This is not as elegant, but is more robust. Whether this is actually a problem will depend on what exceptions can be thrown during construction of the outer object(s). ObjectInputStream can throw IOException which may well get handled by an application without terminating. Many stream classes only throw unchecked exceptions, which may well result in termination of the application.

Hobbie

Further reading: #12553363 – Ferren

Answer 4

It is a good practice to use Apache Commons to handle IO related objects.

In the finally clause use IOUtils

IOUtils.closeQuietly(bWriter); IOUtils.closeQuietly(oWritter);

Code snippet below.

BufferedWriter bWriter = null;
OutputStreamWriter oWritter = null;

try {
  oWritter  = new OutputStreamWriter( httpConnection.getOutputStream(), "utf-8" );
  bWriter = new BufferedWriter( oWritter );
  bWriter.write( xml );
}
finally {
  IOUtils.closeQuietly(bWriter);
  IOUtils.closeQuietly(oWritter);
}

Roundhouse

I added a upvote as I at last discovered a very good way to use Apache Commons. And the template solution from above involves too technical points for me. – Supplication

Oh also, very good tiny example :) – Supplication

Answer 5

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.

Collinsia

I would not count on RuntimeException meaning such an error (Error would imply that). There are developers who believe unchecked exceptions are good for ALL use cases, and others who strongly disagree. – Tipton

Since I'm wiring them up, I know what decorators are in use. In a special case where I'm forced to use a stream that uses RuntimeExceptions incorrectly, I'd have to account for that. Haven't been faced with that yet though. Writing a lot of unnecessarily complicated code to defend against something unlikely and avoidable seems like a bad approach. – Collinsia

RuntimeExceptions are useful for when we can't deal with an exception except several levels up or we need to tunnel through an interface – Philipphilipa

"Bad program" is not a binary condition. A runtime error due to a failure to parse some user input for a single request isn't in the same class as an error that causes complete service failure due to lack of file handles. – Subbasement

@BarryKelly Right, normally you'd want to catch and handle exceptions for bad user input. Using a checked exception there would be appropriate. – Collinsia

Answer 6

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.

Wisecrack

How do you avoid the try? Do you just throw the IOException. But then it won't be closed if an exception occurs? – Philipphilipa

If an exception occurs in the FileOutputStream then rawOut won't be assigned, so we can't do anything, but if the BuffereOutputStream constructor fails we won't be able to close rawOut like we would if we closed if tried closing it first. Additionally, it will crash in the finally as out will be null. – Philipphilipa

That should have been rawOut instead of out in the finally. (Plus a few other typos.) – Wisecrack

Okay, so you close rawOut. Do you close out too, or doesn't it matter? – Philipphilipa

There is no reason to close out. That would be as poinltess as, say, setting it to null. – Wisecrack

Answer 7

The Java SE 7 try-with-resources doesn't seem to be mentioned. It eliminates needing to explicitly do a close completely, and I quite like the idea.

Unfortunately, for Android development this sweet only becomes available by using Android Studio (I think) and targeting Kitkat and above.

Apologia

Answer 8

Also you dont have to close all nested streams

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

Bombard

Link is broken :( (This is why link-only answers are discouraged.) – Promethean

Answer 9

I use to close streams like this, without nesting try-catch in finally blocks

public class StreamTest {

public static void main(String[] args) {

    FileOutputStream fos = null;
    BufferedOutputStream bos = null;
    ObjectOutputStream oos = null;

    try {
        fos = new FileOutputStream(new File("..."));
        bos = new BufferedOutputStream(fos);
        oos = new ObjectOutputStream(bos);
    }
    catch (Exception e) {
    }
    finally {
        Stream.close(oos,bos,fos);
    }
  }   
}

class Stream {

public static void close(AutoCloseable... array) {
    for (AutoCloseable c : array) {
        try {c.close();}
        catch (IOException e) {}
        catch (Exception e) {}
    }
  } 
}

Questionnaire

You are ignoring an Exception in the catch block! – Grappa

Answer 10

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.

Grimsby

I wouldn't count on RuntimeExceptions being documented. In my experience this is only the case for certain RuntimeExceptions that are commonly encounted, such as NumberFormatException. – Linda