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

David Holmes david.holmes at oracle.com
Mon Apr 15 02:56:42 UTC 2013

On 13/04/2013 5:08 AM, Joe Darcy wrote:

On 04/12/2013 11:22 AM, Jason Mehrens wrote:

The landmines are the retrofitted exception classes as shown here https://netbeans.org/bugzilla/showbug.cgi?id=150969 and https://issues.jboss.org/browse/JBREM-552. Really, if the ISE or IAE is thrown it is going to suppress 'this' and 'cause'. It would be nice to see the given 'cause' show up in a log file when tracking down this type of bug. Okay; fair enough. Updated webrev covering initCause too at http://cr.openjdk.java.net/~darcy/8012044.1/ New patch below. (It is a bit of stretch to have this in initiCause by listed as the "cause" of the IllegalStateException; as an alternative, the IllegalStateException could have both this and the cause as suppressed exceptions.)

I don't think that is valid for initCause. If I call initCause twice in succession there need not even be any exception propagation in progress. The ISE that gets thrown is not suppressing anything.

For setSuppressed this might make sense for the compiler generated sequences, but again if I just called setSuppressed with an invalid value, the ISE is not suppressing any existing exception.

David

Cheers,

-Joe --- old/src/share/classes/java/lang/Throwable.java 2013-04-12 12:03:48.000000000 -0700 +++ new/src/share/classes/java/lang/Throwable.java 2013-04-12 12:03:48.000000000 -0700 @@ -452,10 +452,14 @@ * @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); + 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 +1043,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); --- old/test/java/lang/Throwable/SuppressedExceptions.java 2013-04-12 12:03:49.000000000 -0700 +++ new/test/java/lang/Throwable/SuppressedExceptions.java 2013-04-12 12:03:48.000000000 -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 */ @@ -40,6 +40,7 @@ serializationTest(); selfReference(); noModification(); + initCausePlumbing(); } private static void noSelfSuppression() { @@ -48,7 +49,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."); } } @@ -208,4 +211,29 @@ super("The medium.", null, enableSuppression, true); } } + + private static void initCausePlumbing() { + Throwable t1 = new Throwable(); + Throwable t2 = new Throwable("message", t1); + Throwable t3 = new Throwable(); + + try { + t2.initCause(t3); + throw new RuntimeException("Shouldn't reach."); + } catch (IllegalStateException ise) { + if (ise.getCause() != t2) + throw new RuntimeException("Unexpected cause in ISE", ise); + Throwable[] suppressed = ise.getSuppressed(); + if (suppressed.length != 1 || suppressed[0] != t3) + throw new RuntimeException("Bad suppression in ISE", ise); + } + + try { + t3.initCause(t3); + throw new RuntimeException("Shouldn't reach."); + } catch (IllegalArgumentException iae) { + if (iae.getCause() != t3) + throw new RuntimeException("Unexpected cause in ISE", iae); + } + } }

More information about the core-libs-dev mailing list