Closing a Java FileInputStream.

Question

Alright, I have been doing the following (variable names have been changed):

FileInputStream fis = null;
try
{
    fis = new FileInputStream(file);

    ... process ...

     if (fis != null)
        fis.close();
}
catch (IOException e)
{
    ... blah blah blah ...
}

Recently, I started using FindBugs, which suggests that I am not properly closing streams. I decide to see if there's anything that can be done with a finally{} block, and then I see, oh yeah, close() can throw IOException. What the hell are people supposed to do here? The Java libraries throw too many checked exceptions.

UPDATE: Dang, that's what I figured should be done. Blech, oh well.

Answer 1

Something like the following should do it, up to you whether you throw or swallow the IOException on attempting to close the stream.

FileInputStream fis = null;
try
{
    fis = new FileInputStream(file);

    ... process ...


}
catch (IOException e)
{
    ... blah blah blah ...
}
finally
{
    try
    {
        if (fis != null)
            fis.close();
    }
    catch (IOException e)
    {
    }
}

Answer 2

Max's answer is the correct one, and yes, it's annoyingly verbose. You can make it a bit less so by extracting out the contents of a finally clock into a utility method which you can re-use.

In fact, commons-io has a useful IOUtils class that has variouas closeQuietly() methods for just this purpose.

http://commons.apache.org/io/api-release/org/apache/commons/io/IOUtils.html

http://commons.apache.org/io

InputStream fis = null;
try {
    fis = new FileInputStream(file);

    ... process ...


} catch (IOException e) {
    ... blah blah blah ...
} finally {
    IOUtils.closeQuietly(fis);
}

Answer 3

You could also use a simple static Helper Method:

public static void closeQuietly(InputStream s) {
   if (null == s) {
      return;
   }
   try {
      s.close();
   } catch (IOException ioe) {
      //ignore exception
   }
}

and use this from your finally block.

Answer 4

Hopefully we will get closures in Java some day, and then we will lose lots of the verbosity.

So instead there will be a helper method somwhere in javaIO that you can import, it will probably takes a "Closable" interface and also a block. Inside that helper method the try {closable.close() } catch (IOException ex){ //blah} is defined once and for all, and then you will be able to write

 Inputstream s = ....;
 withClosable(s) {
    //your code here
 }

Answer 5

Nothing much to add, except for a very minor stylistic suggestion. The canonical example of self documenting code applies in this case - give a descriptive variable name to the ignored IOException that you must catch on close().

So squiddle's answer becomes:

public static void closeQuietly(InputStream s) {
   try {
      s.close();
   } catch (IOException ignored) {
   }
}

Answer 6

Are you concerned primarily with getting a clean report from FindBugs or with having code that works? These are not necessarily the same thing. Your original code is fine (although I would get rid of the redundant "if (fis != null)" check since an OutOfMemoryException would have been thrown otherwise). FileInputStream has a finalizer method which will close the stream for you in the unlikely event that you actually receive an IOException in your processing. It's simply not worth the bother of making your code more sophisticated to avoid the extremely unlikely scenario of a) you get an IOException and b) this happens so often that you start to run into finalizer backlog issues.

Edit: if you are getting so many IOExceptions that you are running into problems with the finalizer queue then you have far far bigger fish to fry! This is about getting a sense of perspective.

Answer 7

This pattern avoids mucking around with null:

 try {
  InputStream in = new FileInputStream(file);
  try {
   // TODO: work
  } finally {
   in.close();
  }
 } catch (IOException e) {
  // TODO: error handling
 }

---

EDIT: for a more detail on how to effectively deal with close, read this blog post: Java: how not to make a mess of stream handling. It has more sample code, more depth and covers the pitfalls of wrapping close in a catch block.

Answer 8

In most cases, I find it is just better not to catch the IO exceptions, and simply use try-finally:

final InputStream is = ... // (assuming some construction that can't return null)
try {
    // process is
    ...
} finally {
    is.close();
}

Except for FileNotFoundException, you generally can't "work around" an IOException. The only thing left to do is report an error, and you will typically handle that further up the call stack, so I find it better to propagate the exception.

Since IOException is a checked exception, you will have to declare that this code (and any of its clients) throws IOException. That might be too noisy, or you might not want to reveal the implementation detail of using IO. In that case, you can wrap the entire block with an exception handler that wraps the IOException in a RuntimeException or an abstract exception type.

Detail: I am aware that the above code swallows any exception from the try block when the close operation in the finally block produces an IOException. I don't think that is a big problem: generally, the exception from the try block will be the same IOException that causes the close to fail (i.e. it is quite rare for IO to work fine and then fail at the point of closing). If this is a concern, it might be worth the trouble to "silence" the close.