FilterOutputStream.close() throws exception from flush()

[Peter Levart]

On 05/05/2014 11:31 AM, Alan Bateman wrote:
> On 03/05/2014 01:04, Bernd Eckenfels wrote:
>> Hello,
>>
>> back in 2011 there was a discussion about the new changed behavior of
>> FilterOutputStream (and BufferedOutputStream) in regards to not anymore
>> swalloging IOExceptions from flush() on this list (thats where I got
>> the subject from).
>>
>> This was generally a very good improvement (and I am glad that
>> https://bugs.openjdk.java.net/browse/JDK-6335274 thereby got fixed).
>> However the implementation with the try-with-resource has a problem:
>> when flush() and close() report the same exception instance the
>> construction of the suppressed exception will actually fail with an
>> IllegalArgumentException.
>>
>> This IllegalArgumentException of Throwable.addSuppressed is very
>> unfortunate (it would be better simply ignore it).
>>
>> Anyway, this new behavior broke a Unit-Test for Apache VFS as you can
>> see here: https://issues.apache.org/jira/browse/VFS-521
>>
>> I think this can only be fixed in Throwable by avoiding this
>> IllegalArgumentException or by the close() method not using
>> try-with-resource.
>>
>> For reference, according to this changeset other locations are affected
>> as well:
>> http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/759aa847dcaf
>>
> Thanks this bug report, I've created JDK-8042377 to track it. I don't 
> think this has come up before, probably because flush and close are 
> more likely to throw new exceptions rather than equal (cached) 
> exceptions.

Hi Alan,

There has been a discussion about a year ago on this list:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-April/015947.html


>
> I don't see this issue as driver to changing Throwable to support self 
> suppression but we can change BufferedWriter and FilteredOuputStream 
> to only add the suppressed exception when they are not equal.

It's try-with-resources construct that adds suppressed exceptions to 
other exceptions, OuputStreams are just throwing them. And 
(Buffered)FilteredOuputStream doesn't do any exception caching - it just 
propagates exceptions thrown by underlying OuputStream. If underlying 
OuputStream is throwing the same instance on flush() and on close() than 
we have problem. The only thing a FilteredOuputStream could do was to 
actually cache the exception thrown by underlying stream's flush() 
method and compare it by reference with the exception thrown by 
underlying stream's close() method and clone it if it happens to be the 
same instance before throwing it. But that's probably not a very clean 
approach and it would only be effective for FilteredOuputStreams and 
subclasses.

I think it would be better to just change the Throwable.addSuppressed() 
method to treat self-suppression as a no-op. Or change javac to generate 
code that only calls target.addSuppressed(t) when t != target ...

Regards, Peter

>
> -Alan.