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

[Jason Mehrens]

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: jason_mehrens 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(SELF_SUPPRESSION_MESSAGE);
> + throw new
> IllegalArgumentException(SELF_SUPPRESSION_MESSAGE, exception);
>
> if (exception == null)
> throw new NullPointerException(NULL_CAUSE_MESSAGE);
>
> The suppression mechanism is typically, but not exclusively, used by the
> try-with-resources statement.
>
> Thanks,
>
> -Joe