commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Damjan Jovanovic <damjan....@gmail.com>
Subject Re: [imaging] Closing stream
Date Fri, 25 Oct 2013 02:05:50 GMT
On Thu, Oct 24, 2013 at 11:29 PM, Gary Gregory <garydgregory@gmail.com> wrote:
> On Thu, Oct 24, 2013 at 4:31 PM, Jörg Schaible <joerg.schaible@gmx.de>wrote:
>
>> Hi Damjan,
>>
>> Damjan Jovanovic wrote:
>>
>> > As one of the perpetrators of the problem, I have now fixed it. The
>> > reasons I swallowed exceptions were simple:
>>
>> [snip]
>>
>> > * when an exception is thrown and close() then throws another
>> > exception, the close() exception is propagated and the original
>> > exception - which could reveal the cause of the problem - swallowed
>>
>> [snip]
>>
>> And this *is* a real problem. And it is exactly why the IOException of
>> close() are normally ignored. While I don't like
>> IOUtils.closeQietly(closeable), I use normally a method
>> "IO.closeLogged(closeable)" resp. "IO.closeLogged(closeable, logger)".
>>
>> If e.g. an image is corrupted and later on an additional exception occurs
>> closing any resource, you will simply *never* learn about the corrupted
>> image that caused the problem in first case. The original exception must
>> never be swallowed!
>>
>
> Nest'em!
>
> G

What's the way forward though?

1. Catching both exceptions and nesting with setCause():

InputStream is = null;
Exception ex = null;
try {
    is = factoryMethodThatCanThrow();
    is.methodThatCanThrow();
} catch (Exception exx) {
    ex = exx;
} finally {
    if (is != null) {
        try {
            is.close();
        } catch (IOException closeEx) {
            if (ex != null) {
                closeEx.setCause(ex);
            }
            throw closeEx;
        }
    }
}

which only gets worse, as each type of exception has to be separately
caught and rethrown...

2. Swallowing close() exceptions when a succeeded flag at the end of
the try wasn't set:

InputStream is = null;
boolean succeeded = false;
try {
    is = factoryMethodThatCanThrow();
    is.methodThatCanThrow();
    succeeded = true;
} finally {
    if (is != null) {
        try {
            is.close();
        } catch (IOException closeEx) {
            if (succeeded) {
                throw closeEx;
            }
        }
    }
}

which now also requires making sure you don't "return;" before the end
of the try...

3. Java 7 + try-with-resources, which will cripple portability to Android...

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message