On 06/26/2015 09:52 AM, Alan Bateman wrote:
On 25/06/2015 18:58, Brian Burkhalter wrote:
Hi Peter,

Thanks for the code change suggestion. I have modified the patch to use its logic and to revert to the original boolean instance variable instead of the contentious AtomicBoolean:

http://cr.openjdk.java.net/~bpb/8042377/webrev.02/

This looks okay too although your previous version meant that a flush throwing ThreadDeath would cause close to terminate with the error rather than it ending up as the suppressed exception. The new version is better in that it ensures that close is always called and it's probably not worth spending time on the ThreadDeath corner case here.

-Alan.

Quite the contrary, previously, if flush() threw any unchecked exception (ThreadDeath included), it was ignored if out.close() also threw (IOException or any unchecked exception). Latest version at least adds any type of flush() exception to out.close() exception as suppressed. If only one of flush() / out.close() throws any type of exception, then it is propagated as is - previous and latest version does the same...

If some type of unchecked exception from flush() should have precedence over the exception from out.close(), this could be arranged (for example, ThreadDeath):


    public void close() throws IOException {
        if (closed.compareAndSet(false, true)) {
            Throwable flushException = null;
            try {
                flush();
            } catch (Throwable e) {
                flushException = e;
                throw e;
            } finally {
                if (flushException == null) {
                    out.close();
                } else {
                    try {
                        out.close();
                    } catch (Throwable closeException) {

+ // evaluate possible precedence of flushException over closeException
+                       if ((flushException instanceof ThreadDeath) &&
+                           !(closeException instanceof ThreadDeath)) {
+ flushException.addSuppressed(closeException);
+                           throw (ThreadDeath) flushException;
+                       }

                        if (flushException != closeException) {
closeException.addSuppressed(flushException);
                        }
                        throw closeException;
                    }
                }
            }
        }
    }


Regards, Peter

Reply via email to