Thursday, January 17, 2008

Exception handling with ARM blocks

One of the big questions in my mind around ARM blocks is exactly how they should behave when exceptions are thrown while disposing of managed resources. It seems likely that we'd want some flexibility in this - there are probably cases where we'd like the ARM block to suppress these exceptions, but others where that would be inappropriate.

The initial version of the prototype only supported the do() { } form of ARM block, and forced the programmer to catch any checked exceptions thrown by the resources' close() methods, which I've found generally leads to wrapping the do() block in a try { ... } catch (IOException e) { ... }, or adding throws IOException to the enclosing method's signature, neither of which is necessarily useful.

Build 03 changes the default semantics of the do() block so that it no longer throws exceptions raised by calling the close() method, and adds support for a try() form of ARM block which can be used when the programmer does wish to explicitly handle such exceptions.

So in the following example using Closeable resource 'fooFactory' , there's no longer any need to provide explicit handling of IOExceptions:

do (fooFactory) {
System.out.println("innocuous");
}

whilst this code forces us to catch exceptions of type FooException, which is a checked exception:

do (fooFactory) {
Foo foo = fooFactory.createFoo();
if (foo == null)
throw new FooException("no foo :(");
}

and the new try() block allows us to do that more neatly:

try (fooFactory) {
Foo foo = fooFactory.createFoo();
if (foo == null)
throw new FooException("no foo :(");
}
catch (FooException e) {
// something went wrong in the block
}
catch (IOException e) {
// something went wrong disposing of fooFactory
}

A further option for the try() block would be to suppress any exceptions thrown during automatic disposal which are not explicitly caught, allowing us to choose which ones we wish to handle. This doesn't seem quite so nice if the main body of the try block also throws IOExceptions though.

Thoughts welcome.

11 comments:

Josh Bloch said...

Mark,

It was certainly my intention that the programmer not be required to deal with exceptions thrown when terminating a resource: often there's nothing you can do if the dispose fails (e.g., when closing a file that was open for read). But as you say, sometimes you do want to deal with such an exception (e.g., when closing a file that was opened for write, when you're counting on the close to do a flush for you). So I can see why you elected to support both forms. I'm not dead sure of the do vs. try syntax, but I don't have a better idea at the moment either.

    Josh

David Moles said...

I kind of like the try/do dichotomy, but the next obvious language feature that it implies is letting you swap exception-eating do{...} blocks in for ordinary pre-ARM try{...} catch{...} blocks, and that doesn't sound so good. (I mean, I think the feature itself would lead to trouble, but not providing the feature would be confusing.) What if you just allowed do(){...} blocks to have optional catch and finally?

That doesn't solve the other problem, though -- I agree there should be a clear distinction between exceptions thrown by the code in the try(){...} block and exceptions thrown during resource disposal. You could make people put in a separate inner try/catch, but that's a lot of extra curly braces. Plus, the behavior still might not be obvious -- for instance, if the inner code throws an unchecked exception, presumably you don't have to catch it, but if the resource disposal can throw that same unchecked exception, and you decide to catch that one, does that catch block also catch the exception thrown by the inner block? If it does, it's weird, and if it doesn't, it makes it easy to misread the code.

Is there something we can do with finally that would help make the scope more clear? Say, catch for the inner-block exceptions and finally catch for the resource management exceptions?

Mark Mahieu said...

Hi David,

I don't think do(){...} blocks in their current form would normally be a drop-in replacement for ordinary try{...} catch{...} blocks, since they only suppress the exceptions thrown during resource disposal.

Another alternative to supporting catchers on do(){...} blocks would be to allow try(){...} blocks with no catchers and no finally, but I don't see much use in it, unless there's a preference for having just one form of ARM block rather than the two I'm currently supporting.

As for making a distinction between the two 'types' of exception I think you're dead right - neither form works brilliantly for me at the moment for cases like the examples Josh mentioned above.

Probably the worst aspect is that by handling the disposal exceptions in the same catcher that's handling exceptions from the body of a try(){...}, this form encourages the programmer to 'forget' about the disposal exceptions, and write the logic in the catcher as though the exception could only have come from the body, which could lead to subtle bugs - difficult to spot, and difficult to test for.

At the opposite end of the spectrum to what's currently supported with the try(){...} form of ARM block would be allowing resource-specific catchers to be provided. This would give the greatest flexibility, and perhaps the least potential room for error (imagine a block dealing with multiple Input/OutputStreams), but it's probably overkill. I think it would be worth exploring a mid-point between the two extremes, perhaps along the lines you've suggested with finally catch.

David Moles said...

I don't think do(){...} blocks in their current form would normally be a drop-in replacement for ordinary try{...} catch{...} blocks, since they only suppress the exceptions thrown during resource disposal.

Sure. I'm just saying that a new Java programmer, having seen that you can swap do(){...} for try(){...}, would probably want to swap do{...} for try{...} as well, and wonder why it didn't work -- there's an asymmetry there.

...this form encourages the programmer to 'forget' about the disposal exceptions, and write the logic in the catcher as though the exception could only have come from the body.

Totally. Or the other way around: you could call a method in the body that doesn't throw any exceptions, and if later someone else modifies that code so it throws an IOException, you'd never notice.

This would give the greatest flexibility, and perhaps the least potential room for error (imagine a block dealing with multiple Input/OutputStreams), but it's probably overkill.

Yeah, and it's hard to imagine a syntax for that that wouldn't be pretty ugly. On the other hand, if you really need that flexibility, I suppose you can always nest your try(){...} blocks.

Thanks for working on this!

Mark Mahieu said...

...a new Java programmer, having seen that you can swap do(){...} for try(){...}, would probably want to swap do{...} for try{...} as well, and wonder why it didn't work -- there's an asymmetry there.

Fair point.

...you could call a method in the body that doesn't throw any exceptions, and if later someone else modifies that code so it throws an IOException, you'd never notice.

Definitely not good, although that kind of thing can happen already.

Some more thoughts on disposal-exception catchers such as your 'finally catch' idea:

1) If an exception is thrown from the body of a try(){...} and another subsequently thrown as a result of disposing a resource, obviously we want a catch block for the first to be executed if provided, but should the disposal-catch block also be executed afterwards to handle the disposal exception? That would be technically consistent with what happens if you code it as nested try{...} blocks in Java 6, but whether it makes sense once you collapse all this into a single try(){...} ARM block... I suspect it wouldn't, but it depends on the structure of the block, and the implications in terms of what the programmer naturally expects.

2) Does it still make most sense for the resources to be disposed immediately after the main body has completed, or should that happen after the 'normal' catchers have been executed? Similar question regarding the finally block, if it's allowed.

3) So far I've only implemented Closeable resources, but if ARM blocks were to support resource initialization as well, what impact does this have on exception handling?

Yeah, and it's hard to imagine a syntax for that that wouldn't be pretty ugly. On the other hand, if you really need that flexibility, I suppose you can always nest your try(){...} blocks.

Agreed. This doesn't look too bad though, to me anyway:

try (FileInputStream in = openForRead(inputFile);
      FileOutputStream out = openForWrite(outputFile)) {
        // do stuff with in and out
}
catch (IOException ex) {
        // handle any IOExceptions thrown in body of try() block
}
catch (IOException ex, OutputStream fos) {
        // specifically handle IOExceptions thrown
        // as a result of close()ing an OutputStream
        // resource (ie. 'out')
}

David Moles said...

Hmm, no, that's not too bad. Could lead to trouble if you have more than one resource of the same type, though.

Bharath said...

Mark,
How about mandating that the Exceptions arising out of closing the resource can be the only ones caught after the try(){} block, and that other exceptions thrown within that try(){} block cannot be caught outside the block?
That way, if an application specific IOException needs to be caught, a conventional try-catch could be used within the outer try(){} block [not very elegant, I agree].
Would that make sense? Just a thought.

By the way, please let me know (through a comment here) if you need any assistance in further implementing the CICE/ARM proposal.
It might also help if your current work is put up at a place like ksl or kijaro with an OpenJDK-compatible license for others to contribute or provide feedback.

Thanks,
Bharath

sicklittlemonkey said...

I'm a bit late to the party, but why are all the proposals so C-like in their eagerness to declare managed variables at the beginning of a block?

This saves a few keystrokes on the pre-Java 7 equivalent but is needlessly verbose IMO. Why not declare ARM variables anywhere marked with a keyword, and let the compiler do the work for us (like C++ does).

So this:
do (fooFactory) {
Foo foo = fooFactory.createFoo();
}

becomes somthing like this:
transient Foo foo = fooFactory.createFoo();

I've chosen to re-use transient here rather than a new keyword (e.g. local) since overloading it creates no abiguities.

Cheers,
Nick.

vikas said...

Well, I was hoping to replace:
try
{
Scanner in = new Scanner(new FileInputStream(file));
while (in.hasNextLine())
{
//doSomething
}
}
catch (IOException ex)
{ex.printStackTrace();}

with:
do(Scanner in = new Scanner(new FileInputStream(file)))
{
while (in.hasNextLine())
{
//doSomething
}
}

But turns out I need to handle FileNotFoundExceptions...so it doesnt make the syntax prettier although I use the do() :(.

Will 'Varfar' said...

1) Python's "with" keyword for ARM is much nicer than overloading the "do", imo.

2) its bad form for something that is called in cleanup to itself throw. The "closeable" function signature should explicitly not throw *anything* ever.

Muriel said...

The thing you're writing is a big blunder.