46

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?

Austin Hyde
  • 26,347
  • 28
  • 96
  • 129
  • 6
    Note that your knee-jerk fix won't close `fos` when `oos.close()` throws `IOException`. Each needs to go in its own try-catch. `if (oos != null) try { oos.close() } catch (IOException logOrIgnore) {}` and so on. – BalusC Nov 04 '10 at 00:57
  • 3
    @BalusC - that's a good point. Thanks for ...catching... that :D – Austin Hyde Nov 04 '10 at 01:01
  • 1
    Actually, oos.close() closes the underlying fos too. – SimonJ Nov 04 '10 at 01:19
  • In C# you would use a `using`, but any exceptions thrown from the Dispose method on the IDisposable would just bubble up, so the equivalent Java would be to throw a RuntimeException on the catch of the close statement. But I don't know if the .NET system libraries swallow such exceptions by default or not. – Yishai Nov 04 '10 at 01:53
  • There should be a canonical question for this -- there are a raft of very similar questions, and a searcher/reader has a hard time figuring out which to follow. Can some moderator type figure out how to merge some of these questions? – andersoj Nov 04 '10 at 02:01
  • See also http://blogs.sun.com/darcy/entr/project_coin_updated_arm_spec – andersoj Nov 04 '10 at 02:03

8 Answers8

60

Note that the following is only applicable for Java 6 and earlier. For Java 7 and later, you should switch to using try-with-resources ... as described in other answers.

If you are trying to catch and report all exceptions at source (in Java 6 or earlier), 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.

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

UPDATE : closeQuietly is deprecated in version 2.6 of the Apache Commons API. Java 7+ try-with-resources makes it redundant.


On the issue of flush() versus close() that people were asking about in comments:

  • 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 ...)

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • 1
    +1 for pointing out the flush issue, it is the most often missed issue. – Yishai Nov 04 '10 at 01:44
  • Is it not a good idea of flushing the object in finally block ? – Little bird May 15 '15 at 13:10
  • @Littlebird - It is pointless, It is the responsibility of the `close()` method to flush ... if it is a meaningful thing to do. – Stephen C May 15 '15 at 14:02
  • The IOUtils.closeQuietly is doing exactly what you suggest. This is a dependency I generaly use inside my Maven projects (commons-io from Appache available here: https://commons.apache.org/proper/commons-io/) – A. Masson Feb 17 '16 at 15:55
32

Current best practice for try/catch/finally involving objects that are closeable (e.g. Files) is to use Java 7's try-with-resource statement, e.g.:

try (FileReader reader = new FileReader("ex.txt")) {
    System.out.println((char)reader.read());
} catch (IOException ioe) {
    ioe.printStackTrace();
}

In this case, the FileReader is automatically closed at the end of the try statement, without the need to close it in an explicit finally block. There are a few examples here:

http://ppkwok.blogspot.com/2012/11/java-cafe-2-try-with-resources.html

The official Java description is at:

http://docs.oracle.com/javase/7/docs/technotes/guides/language/try-with-resources.html

Phil
  • 371
  • 4
  • 2
16

Java 7 will add Automatic Resource Management blocks. They are very similar to C#'s using.

Josh Bloch wrote the technical proposal, which I highly recommend reading. Not just because it will give you a leg up on an upcoming Java 7 language feature, but because the specification motivates the need for such a construct, and in doing so, illustrates how to write correct code even in the absence of ARM.

Here's an example of the Asker's code, translated into ARM form:

try (FileOutputStream fos = new FileOutputStream(file);
        ObjectOutputStream oos = new ObjectOutputStream(fos)) 
{
    oos.writeObject(shapes);
}
catch (FileNotFoundException ex) 
{
    // handle the file not being found
}
catch (IOException ex) 
{
    // handle some I/O problem
}
Mike Clark
  • 10,027
  • 3
  • 40
  • 54
  • Will ARM also call `flush();` before `close();`? –  Jan 12 '17 at 21:12
  • 2
    @Muddz No, ARM will not call flush. It is worth noting that most common Java classes being used in common ways do not require you to call flush() before calling close(), as the lower-level implementation of close() generally does enough work to make an explicit end-of-stream flush unnecessary. Unfortunately this flush/close contract is not documented, it's just sort of a known implicit behavior that people generally trust. In specific cases where you know you need to call flush() before close(), you could decorate your stream, e.g. `new MyFlushOnCloseOutputStream(originalOutputStream)` . – Mike Clark Jan 12 '17 at 22:00
  • Allright well than it saves me from worrying about it. Thanks for the explanation! –  Jan 12 '17 at 22:04
4

I usually have small class IOUtil with method such as:

public static void close(Closeable c) {
    if (c != null) {
        try {
            c.close();
        }
        catch (IOException e) {
            // ignore or log
        }
    }
}
maximdim
  • 8,041
  • 3
  • 33
  • 48
  • 2
    This is definitely my preferred approach. I always make sure to log an Exception on closing. It may be a sign of something nasty happening. Although it shouldn't stop normal operation, I really feel an error here should be logged somewhere. – Dunderklumpen Nov 04 '10 at 02:06
3

How about this guys? No null check, no surprise. Everything is cleaned upon exit.

try {
    final FileOutputStream fos = new FileOutputStream(file);
    try {
        final ObjectOutputStream oos = new ObjectOutputStream(fos);
        try {
            oos.writeObject(shapes);
            oos.flush();
        }
        catch(IOException ioe) {
            // notify user of important exception
        }
        finally {
            oos.close();
        }
    }
    finally {
        fos.close();
    }
}
catch (FileNotFoundException ex) {
    // complain to user
}
catch (IOException ex) {
    // notify user
}
RAY
  • 6,810
  • 6
  • 40
  • 67
  • No - an exception thrown by close will replace the probably more informative exception thrown by the actual file operations. – Lawrence Dol Nov 04 '10 at 02:51
  • you mean exception from fos.close() eclipsing exception from oos.writeObject()? Let's catch it and handle it then. (original post edited) – RAY Nov 04 '10 at 03:07
  • 1
    RAY's code is the closest to what Java 7 ARM would write for you. Except, Java 7 ARM will remember any exception encountered in try{} and will ultimately throw it in preference to any possible exceptions that might have occurred in finally{}. – Mike Clark Nov 04 '10 at 11:33
  • +1, except I'm not sure you even need the catch in the innermost try, assuming you are handling both IOException blocks the same way. – ILMTitan Nov 04 '10 at 15:53
  • @ILMTitan. Thanks! The innermost try was added per Software Monkey's comment. Please correct me if I'm wrong, my understanding of his comment is that if both oos.writeObject() and fos.close() throws an exception, then oos.writeObject() will be lost and not handled. – RAY Nov 05 '10 at 01:36
  • @RAY true, but if you don't care what method caused the IOException, then you don't need it. I've never been quite sure why people would want to treat those two cases so differently. – ILMTitan Nov 05 '10 at 14:32
  • Will ARM also call `flush();` before `close();`? @RAY –  Jan 12 '17 at 21:12
1

Unfortunately there is no language level support. But there are lot of libraries out there that makes this simple. Check commons-io library. Or modern google-guava @ http://guava-libraries.googlecode.com/svn/trunk/javadoc/index.html

Aravind Yarram
  • 78,777
  • 46
  • 231
  • 327
  • 3
    More specifically, Closeables.closeQuietly which is documented here: http://guava-libraries.googlecode.com/svn/trunk/javadoc/com/google/common/io/Closeables.html#closeQuietly(java.io.Closeable) – SimonJ Nov 04 '10 at 00:43
  • +1 @SimonJ for useful link. The more I read stackoverflow, the more it appears guava is a Really Useful Thing (tm). – orangepips Nov 04 '10 at 00:49
1

You're doing it right. It bothers the crap out of me too. You should initialize those streams to null explicitly - it's common convention. All you can do is join the club and want for using.

Mike
  • 19,267
  • 11
  • 56
  • 72
1

Not a direct answer to your point but it is an unfortunate fact that because finally and catch are both associated with try people think that they belong together. The best design for try blocks is to either have a catch or a finally but not both.

In this case your comments hint that something is wrong. Why, in a method dealing with file IO, are we complaining about anything to the user. We might be running deep on a server somewhere with nary a user in sight.

So, the code you present above should have a finally to fail gracefully when things go wrong. It lacks the capacity to deal intelligently with errors however, so your catch belongs somewhere higher up on the call chain.

CurtainDog
  • 3,175
  • 21
  • 17
  • 2
    Disagree with the "either but not both" part; I have plenty of examples where I needed to perform some exception handling, but absolutely must have a finally to clean up resources. – Lawrence Dol Nov 04 '10 at 00:50
  • Normally, I'd agree with you 100%, and I'm all about maintaining proper responsibilities and what-not, but this is a quick little school project with a simple UI (one class) that is meant to get us to learn about file IO in Java. You should be glad I didn't put all that code in the event handler in an anonymous ActionListener! :D – Austin Hyde Nov 04 '10 at 00:53
  • @Software Monkey - Fair call, I don't mean to imply that you shouldn't have finally blocks, quite the reverse in fact. It's just that by using either a finally block or catch block(s) you get the same semantics as C#'s using block, though if the exceptions are checked you will have to explicity mark the method as throwing them. – CurtainDog Nov 04 '10 at 00:56
  • @Austin Hyde - The quick and dirty way would be to nest your `try...finally` inside your `try...catch` – CurtainDog Nov 04 '10 at 00:58
  • 1
    This does not deserve a downvote, quite often the double try is the correct idiom, even if most developers don't use it. See here: http://illegalargumentexception.blogspot.com/2008/10/java-how-not-to-make-mess-of-stream.html – Yishai Nov 04 '10 at 01:31