Hi,
Concerning Java 7, I think that the try-with-ressources throws the
first exception encountered, and add other exceptions in the
"suppressed" exceptions.
See
http://docs.oracle.com/javase/7/docs/api/java/lang/Throwable.html#addSuppressed(java.lang.Throwable)
My 2 cents,
Regards,
Julien
2013/10/25 Jörg Schaible <joerg.schai...@scalaris.com>:
> Hi Damjan,
>
> Damjan Jovanovic wrote:
>
>> On Thu, Oct 24, 2013 at 11:29 PM, Gary Gregory <garydgreg...@gmail.com>
>> wrote:
>>> On Thu, Oct 24, 2013 at 4:31 PM, Jörg Schaible
>>> <joerg.schai...@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...
>
>
> Right, you could have got even an OOME reading an image ... :-/
>
>
>> 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...
>
>
> IMHO this is the best approach so far. Maybe we can introduce a utility
> method somewhere to avoid the boilerplate copies:
>
> =============== %< ==================
> public static void closeSafely(boolean mayThrow, Closeable... closeables)
> throws IOException {
> IOException ioex = null;
> for(Closeable closeable : closeables) {
> if (closeable != null) {
> try {
> closeable.close();
> } catch (IOException ex) {
> ioex = ex; // keep first or last ?!?
> }
> }
> }
> if (mayThrow && ioex != null) {
> throw ioex;
> }
> }
> =============== %< ==================
>
>
>> 3. Java 7 + try-with-resources, which will cripple portability to
>> Android...
>
> I would first write a unit test to see what Java 7 actually does with
> multiple open resources in the different error cases.
>
> Cheers,
> Jörg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
