Monday 6 April 2009

Java Resource Handling: Throwing the correct error

To recap - we want to create a resource, do some processing with it, then close it. If it throws an exception, then it must get closed and propogate the first exception encountered.

Naive Approach

The call to close() in our code must be within a try block (for the occasions where it needs to be supressed), and then the code gets quite complicated, e.g:

void foo() throws IOException {
        IOException exception = null;
        SomeResource r1 = null;
        try {
            //create and use r1
        }
        catch(IOException e) {
            exception=e;
        }
        try {
            if(r1!=null) r1.close();
        }
        catch(IOException closeException) {
            if(exception==null) {
                throw closeException;
            }
        }
        if(exception!=null) {
            throw exception;
        }
    }

Not nice!

Alternative #1

How about we have more than one call to close()? Then the code can be quite significantly simplified:

void foo() throws IOException {
        SomeResource r1=null;
        try {
            //create and use r1
            r1.close();
        }
        finally {
            try {
                if(r1!=null) r1.close;
            }
            catch(IOException e) { /*swallow error*/ }
        }
    }
 
This relies on it being legal to call close() multiple times (we can always have a flag to stop it being called twice, which is easy to do if it's wrapped up in a framework).
The obvious extra simplification is to write a closeSilently function:

void CloseSilently(Closeable c) {
    try {
        if(c!=null) c.close();
    }
    catch(IOException e) { /*swallow error*/ }
}

which enables us to write:

void foo() throws IOException {
    SomeResource r1=null;
    try {
        //create and use r1
        r1.close();
    }
    finally {
        closeSilently(r1);
    }
}

So let us critique this solution - slight errors in this code can lead to the worse class of defect (see my first post) - omit the first call to close() and the code will appear to work correctly until close() throws an error which is silently ignored! You don't need nested try blocks to handle multiple resources, but you do need to duplicate all the calls to close() in the same order as calls to closeSilently(). This can be solved by means of a helper framework, but still the deadly silent error is a real problem...

Alternative #2

We used a try/catch + try/catch set-up in the Naive Approach in order to detect if the resource-using code threw an exception or not. Catch isn't the only way to do; note that the last line of the try block runs if and only if no exception was thrown, and we can use this fact instead as follows:

void foo() throws IOException {
    boolean tryBlockCompleted = false;
    SomeResource r1=null;
    try {
        //create and use r1
        tryBlockCompleted = true;
    }
    finally {
        if(tryBlockCompleted) {
            r1.close();
        } else {
            CloseSilently(r1);
        }
    }
}

A helper class would make life a lot easier here:

void foo() throws IOException {
    Closer closer = new Closer();//the new helper class
    SomeResource r1=null;
    try {
        //create and use r1
        closer.completed();
    }
    finally {
        closer.close(r1);
    }
}

In Closer::close() we can assert that completed() has been called, hence in the unit tests we can ensure that the code is used correctly :-)
How about multiple resources?

void foo() throws IOException {

    Closer closer = new Closer();//the new helper class
    SomeResource r1=null;
    SomeResource r2=null;
    try {
        //create and use r1 & r2
        closer.completed();
    }
    finally {
        closer.close(r2,r1);
    }
}

Not particularly complex :-)

Alternative #3

We have a minor variation to alternative 2 above - do we have to make closing in the correct order explicit (as above), or add each closable item to closer as it is created and rely upon it to get the order correct? E.g.: 


void foo() throws IOException {
    Closer closer = new Closer();//the new helper class
    try {
        SomeResource r1 = ...
        closer.add(r1);
        SomeResource r2 = ...
        closer.add(r2);
//use r1 & r2
        closer.completed();
    }
    finally {
        closer.close();
    }
}

This alternative, whilst attractive, isn't actually any shorter - and omitted calls to Closer::add() could lead to silent runtime errors.

Alternative #4

Up until now the helping frameworks discussed have been classes that have been used (or functions that have been called) from the resource-handling code. There is an alternative approach, Inversion of control, where we pass the framework the resource-handling code we wish to run and it calls our code from within its own error handling & close calling code. In Java 6 that means creating an anonymous inner class containing our resource-handling code and passing it to the framework, e.g.:

void foo() throws IOException {
    String result = new ResourceHandler {
        SomeResource r1=...;
        add(r1);
        SomeResource r2=...;
        add(r2);
    }.execute();
}

This is quite attractively short, but looks confusing and still has the silent defect problem associated with Alternative #3 if we omit a call to add()
(See link for a paper describing a very similar approach to this).

Alternative #5

How about we change the rules, and ask for multiple exceptions to be thrown when multiple exceptions are thrown; i.e. when an exception thrown results in multiple extra exceptions when closing the resources, throw all of them.
See link for an example of this approach. This example is over-complex and the bolier-plate code can easily be hidden as per Alternatives 2,3 & 4, but this ignores the extra burden laid upon the client. Handling the array of exceptions thrown is rather over-complicated, and to what end?

Summary

Of the various solutions presented, the only one which appears to be safe to use is alternative #2, the others are too prone to silent error, or are pointlessly over-complicated.

2 comments:

  1. Alternative #2 is incorrect when the block of code using the resource contains a break, continue, or return statement that exits the try statement.

    ReplyDelete
  2. There's not a problem exiting the block with a break/continue/return statement - you simply have to call closer.completed() before that. If you forget to do so then it will assert when that codepath executes.

    ReplyDelete