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