Re: Returning external resources and Exceptions

From: Gerald Thaler (gerald.thaler_at_gmx.de)
Date: 12/14/04 

Date: Tue, 14 Dec 2004 22:17:32 +0100

Patricia Shanahan wrote:
> OK, that clarifies the requirements. Here's one approach. I suggest adding
> a boolean variable exceptionExists, initially false.
>
> As you go through the exception handling, catch each IOException,
> including ones in the original try block, with a block like this:
>
> catch(IOException e){
> if(!exceptionExists){
> exceptionExists = true;
> throw e;
> }
> }
>
> The first catch block to see an exception sets exceptionExists true, and
> completes abruptly throwing the exception. Subsequent catch blocks
> complete normally, preserving the first exception.

This all results in very complicated control flow. Ist doesn't seem correct
either: If the first cleanup action inside the finally clause throws, the
rest of the finally is never executed.

I worked out several solutions. None of them is "elegant". They all have
some ugly code duplication. I first wrote a helper method that doesnt throw
to somewhat reduce the clutter:

public static void safeClose(Closeable c) {
    if (c != null) {
        try { c.close(); } catch (IOException e) { }
    }
}

Then one could do the following:

public void convert(File fileIn, File fileOut) throws IOException {
    FileInputStream in = null;
    FileOutputStream out = null;
    try {
        in = new FileInputStream(fileIn); // may throw
        out = new FileOutputStream(fileOut); // may throw
        // read, compute, write // may throw
        in.close(); // may throw
        in = null; // (1)
        out.close(); // may throw
        out = null; // (2)
    } finally {
        safeClose(in);
        safeClose(out);
    }
}

Note that the lines (1) and (2) are not really necessary, as multiple
Closeable.close() calls dont do any harm. Anyway, the finally block here
does nothing useful unless an exception is thrown from the try block. This
perverts the intention of 'finally'.

So we may as well replace the finally by a catch block:
        ...
        in.close(); // may throw
        in = null; // (1)
        out.close(); // may throw
        out = null; // (2)
    } catch (IOException e) {
        safeClose(in);
        safeClose(out);
        throw e;
    }
}

Now the code is somewhat clearer: The safeClose()-calls are only executed
when an exception is thrown. One could also perform some additional cleanup
(deleting the output file) before rethrowing the exception in this case.

Nevertheless, there remains the problem of code duplication. IMHO the root
cause of the whole debacle is that the close() methods in the API can throw.
The same is true for most other cleanup methods in the API: Socket.close(),
java.sql.Connection.close() ...

This seems to be a serious design flaw in the API. Cleanup code that throws
is no good. It renders the finally keyword almost useless. 'finally' was
intended primarily as a means for reliable cleanup of external Resources
without code duplication. But it's incompatible with the API. 'finally'
fails big time if the cleanup code itself can throw.

It's only remotely usefull if you have just one resource to cleanup:

} finally {
    if (out != null)
        out.close();
}

But even then there is a slight problem, as the out.close() call may throw
and hide the original exception (the root cause of the problem) from the
try-body. When there is more than one resource to take care of, 'finally'
doesn't help at all if you want to write 100% reliable code. Worse, it even
seems to fool some developers into writing broken code that will swallow
exceptions. I wonder wether there exists an official working
finally-idiom/pattern!?

Relevant Pages

  • Re: Returning external resources and Exceptions
    ... > when an exception is thrown. ... there remains the problem of code duplication. ... The same is true for most other cleanup methods in the API: ...
    (comp.lang.java.help)
  • Re: Operation can be dispatching in only one type
    ... exception), as Program_Error always represents a program bug. ... cleanup) usually represents a bad design. ... use anonymous access types. ... whether a check will fail is known statically for any compiler ...
    (comp.lang.ada)
  • Re: A "scopeguard" for Python
    ... Revert to the original state. ... For chdir(), it really doesn't matter. ... fails, and chdiris essentially atomic so if it raises an exception, ... cleanup can frequently be written to not care ...
    (comp.lang.python)
  • Re: A "scopeguard" for Python
    ... Do stuff requiring the modified state. ... Revert to the original state. ... fails, and chdiris essentially atomic so if it raises an exception, ... cleanup can frequently be written to not care ...
    (comp.lang.python)
  • Re: Why finally?
    ... other than organizational aesthetics ?? ... First of all, its not very common, or shouldn't be common to use catch all ... coders know that any cleanup will be within the finally block. ... An exception is raised in the catch block, finally will be executed, your ...
    (microsoft.public.dotnet.languages.csharp)