Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed (original) (raw)

Jason Mehrens jason_mehrens at hotmail.com
Wed Apr 17 21:03:20 UTC 2013


Joe,

The patch at http://cr.openjdk.java.net/~darcy/8012044.3/ looks good to me. Thanks for working on this.

Jason

Date: Wed, 17 Apr 2013 10:32:10 -0700 From: joe.darcy at oracle.com To: jasonmehrens at hotmail.com CC: core-libs-dev at openjdk.java.net; David.Holmes at oracle.com Subject: Re: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

On 04/14/2013 07:36 PM, Joe Darcy wrote: > On 04/12/2013 07:29 PM, Jason Mehrens wrote: >> Joe, >> You'll have guard ise.addSuppressed against null. Looks good otherwise. >> >> private static void initCauseNull() { >> Throwable t1 = new Throwable(); >> t1.initCause(null); >> try { >> t1.initCause(null); >> } catch(IllegalStateException expect) { >> } >> } > > Right you are; check added and test updated in: > > http://cr.openjdk.java.net/~darcy/8012044.2/ > > Full patch to Throwable: [snip] One more iteration; I've changed the initCause logic to suppress both exceptions rather than using one as the cause: http://cr.openjdk.java.net/~darcy/8012044.2 Patch to throwable: --- old/src/share/classes/java/lang/Throwable.java 2013-04-14 19:33:23.000000000 -0700 +++ new/src/share/classes/java/lang/Throwable.java 2013-04-14 19:33:23.000000000 -0700 @@ -452,10 +452,15 @@ * @since 1.4 */ public synchronized Throwable initCause(Throwable cause) { - if (this.cause != this) - throw new IllegalStateException("Can't overwrite cause"); + if (this.cause != this) { + IllegalStateException ise = + new IllegalStateException("Can't overwrite cause", this); + if (cause != null) + ise.addSuppressed(cause); + throw ise; + } if (cause == this) - throw new IllegalArgumentException("Self-causation not permitted"); + throw new IllegalArgumentException("Self-causation not permitted", this); this.cause = cause; return this; } @@ -1039,7 +1044,7 @@ */ public final synchronized void addSuppressed(Throwable exception) { if (exception == this) - throw new IllegalArgumentException(SELFSUPPRESSIONMESSAGE); + throw new IllegalArgumentException(SELFSUPPRESSIONMESSAGE, exception); if (exception == null) throw new NullPointerException(NULLCAUSEMESSAGE); The suppression mechanism is typically, but not exclusively, used by the try-with-resources statement. Thanks, -Joe



More information about the core-libs-dev mailing list