RAII in Java... is resource disposal always so ugly ? (was: I had a dream ?)

Question

Hi all,

I just played with Java file system API, and came down with the following function, used to copy binary files. The original source came from the Web, but I added try/catch/finally clauses to be sure that, should something wrong happen, the Buffer Streams would be closed (and thus, my OS ressources freed) before quiting the function.

I trimmed down the function to show the pattern:

public static void copyFile(FileOutputStream oDStream, FileInputStream oSStream) throw etc...
{
   BufferedInputStream oSBuffer = new BufferedInputStream(oSStream, 4096);
   BufferedOutputStream oDBuffer = new BufferedOutputStream(oDStream, 4096);

   try
   { 
      try
      { 
         int c;

         while((c = oSBuffer.read()) != -1)  // could throw a IOException
         {
            oDBuffer.write(c);  // could throw a IOException
         }
      }
      finally
      {
         oDBuffer.close(); // could throw a IOException
      }
   }
   finally
   {
      oSBuffer.close(); // could throw a IOException
   }
}

As far as I understand it, I cannot put the two "close()" in the finally clause because the first "close()" could well throw, and then, the second would not be executed.

I know C# has the Dispose pattern that would have handled this with the using keyword.

I even know better a C++ code would have been something like (using a Java-like API):

void copyFile(FileOutputStream & oDStream, FileInputStream & oSStream)
{
   BufferedInputStream oSBuffer(oSStream, 4096);
   BufferedOutputStream oDBuffer(oDStream, 4096);

   int c;

   while((c = oSBuffer.read()) != -1)  // could throw a IOException
   {
      oDBuffer.write(c);  // could throw a IOException
   }

   // I don't care about resources, as RAII handle them for me
}

I am missing something, or do I really have to produce ugly and bloated code in Java just to handle exceptions in the close() method of a Buffered Stream?

(Please, tell me I'm wrong somewhere...)

EDIT: Is it me, or when updating this page, I saw both the question and all the answers decreased by one point in a couple of minutes? Is someone enjoying himself too much while remaning anonymous?

EDIT 2: McDowell offered a very interesting link I felt I had to mention here: http://illegalargumentexception.blogspot.com/2008/10/java-how-not-to-make-mess-of-stream.html

EDIT 3: Following McDowell's link, I tumbled upon a proposal for Java 7 of a pattern similar to the C# using pattern: http://tech.puredanger.com/java7/#resourceblock . My problem is explicitely described. Apparently, even with the Java 7 "do", the problems remain.

Answer 1

Unfortunately, this type of code tends to get a bit bloated in Java.

By the way, if one of the calls to oSBuffer.read or oDBuffer.write throws an exception, then you probably want to let that exception permeate up the call hierarchy.

Having an unguarded call to close() inside a finally-clause will cause the original exception to be replaced by one produced by the close()-call. In other words, a failing close()-method may hide the original exception produced by read() or write(). So, I think you want to ignore exceptions thrown by close() if and only if the other methods did not throw.

I usually solve this by including an explicit close-call, inside the inner try:

  try {
    while (...) {
      read...
      write...
    }
    oSBuffer.close(); // exception NOT ignored here
    oDBuffer.close(); // exception NOT ignored here
  } finally {
    silentClose(oSBuffer); // exception ignored here
    silentClose(oDBuffer); // exception ignored here
  }

  static void silentClose(Closeable c)  {
    try {
      c.close();
    } catch (IOException ie) {
      // Ignored; caller must have this intention
    }
  }

Finally, for performance, the code should probably work with buffers (multiple bytes per read/write). Can't back that by numbers, but fewer calls should be more efficient than adding buffered streams on top.

Answer 2

Your try/finally pattern is the correct way to handle streams in most cases.

Some are advocating silently closing streams. Do not do it for these reasons: Java: how not to make a mess of stream handling

Answer 3

There are issues, but the code you found lying about on the web is really poor.

Closing the buffer streams closes the stream underneath. You really don't want to do that. All you want to do is flush the output stream. Also there's no point in specifying the underlying streams are for files. Performance sucks because you are copying one byte at a time (actually if you use java.io use can use transferTo/transferFrom which is a bit faster still). While we are about it, the variable names suck to. So:

public static void copy(
    InputStream in, OutputStream out
) throw IOException {
    byte[] buff = new byte[8192];
    for (;;) {
        int len = in.read(buff);
        if (len == -1) {
            break;
        }
        out.write(buff, 0, len);
    }
}

If you find yourself using try-finally a lot, then you can factor it out with the "execute around" idiom.

In my opinion: Java should have someway of closing resources at end of scope. I suggest adding private as a unary postfix operator to close at the end of the enclosing block.

Answer 4

Yes, that's how java works. There is control inversion - the user of the object has to know how to clean up the object instead of the object itself cleaning up after itself. This unfortunately leads to a lot of cleanup code scattered throughout your java code.

C# has the "using" keyword to automatically call Dispose when an object goes out of scope. Java has no such thing.

Answer 5

For common IO tasks such as copying a file, code such as that shown above is reinventing the wheel. Unfortunately, the JDK doesn't provide any higher level utilities, but apache commons-io does.

For example, FileUtils contains various utility methods for working with files and directories (including copying). On the other hand, if you really need to use the IO support in the JDK, IOUtils contains a set of closeQuietly() methods that close Readers, Writers, Streams, etc. without throwing exceptions.