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

Peter Levart peter.levart at gmail.com
Fri Apr 12 07:01:34 UTC 2013

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(SELFSUPPRESSIONMESSAGE); + throw new IllegalArgumentException(SELFSUPPRESSIONMESSAGE, exception); if (exception == null) throw new NullPointerException(NULLCAUSEMESSAGE); 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

More information about the core-libs-dev mailing list