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

[Peter Levart]

Hi Joe,

There were certainly debates about why self-suppression is not a good 
thing when project Coin's try-with-resources has been developed, but I 
don't quite remember the reason why it was designed this way. Couldn't 
the logic just make the self-suppression a no-op? The addSuppressed was 
designed for (quoting from javadoc):

"...there are situations where two independent exceptions can be thrown 
in sibling code blocks, in particular in the try block of a 
try-with-resources statement and the compiler-generated finally block 
which closes the resource. In these situations, only one of the thrown 
exceptions can be propagated..."

Now if those "two independent exceptions" are actually the same 
exception, there's no "dilemma"...


Regards, Peter


On 04/12/2013 03:19 AM, Joe Darcy wrote:
> Hello,
>
> Please review the patch below to address
>
>     8012044: Give more information about self-suppression from 
> Throwable.addSuppressed
>     http://cr.openjdk.java.net/~darcy/8012044.0/
>
> Thanks,
>
> -Joe
>
> diff -r 006a7a576fe9 src/share/classes/java/lang/Throwable.java
> --- a/src/share/classes/java/lang/Throwable.java    Thu Apr 11 
> 12:22:23 2013 +0900
> +++ b/src/share/classes/java/lang/Throwable.java    Thu Apr 11 
> 18:16:38 2013 -0700
> @@ -1039,7 +1039,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);
> diff -r 006a7a576fe9 test/java/lang/Throwable/SuppressedExceptions.java
> --- a/test/java/lang/Throwable/SuppressedExceptions.java    Thu Apr 11 
> 12:22:23 2013 +0900
> +++ b/test/java/lang/Throwable/SuppressedExceptions.java    Thu Apr 11 
> 18:16:38 2013 -0700
> @@ -26,7 +26,7 @@
>
>  /*
>   * @test
> - * @bug     6911258 6962571 6963622 6991528 7005628
> + * @bug     6911258 6962571 6963622 6991528 7005628 8012044
>   * @summary Basic tests of suppressed exceptions
>   * @author  Joseph D. Darcy
>   */
> @@ -48,7 +48,9 @@
>              throwable.addSuppressed(throwable);
>              throw new RuntimeException("IllegalArgumentException for 
> self-suppresion not thrown.");
>          } catch (IllegalArgumentException iae) {
> -            ; // Expected
> +            // Expected to be here
> +            if (iae.getCause() != throwable)
> +                throw new RuntimeException("Bad cause after 
> self-suppresion.");
>          }
>      }
>
>
> -Joe