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

[Joseph Darcy]

Hello,

Just reasserting the request for a review of the latest version of this 
patch:

     http://cr.openjdk.java.net/~darcy/8012044.2

I believe this version does an appropriate job of propagating exception 
information when there is misuse of the methods on Throwable.

Thanks,

-Joe

On 4/17/2013 10:32 AM, Joe Darcy wrote:
> 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