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) {
}
}
Jason
Date: Fri, 12 Apr 2013 12:08:07 -0700
From: joe.da...@oracle.com
To: jason_mehr...@hotmail.com
CC: core-libs-dev@openjdk.java.net
Subject: Re: Code review request for 8012044: Give more information about
self-suppression from Throwable.addSuppressed
On 04/12/2013 11:22 AM, Jason Mehrens wrote:
The landmines are the retrofitted exception classes as shown here
https://netbeans.org/bugzilla/show_bug.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.)
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(SELF_SUPPRESSION_MESSAGE);
+ throw new IllegalArgumentException(SELF_SUPPRESSION_MESSAGE, exception);
if (exception == null)
throw new NullPointerException(NULL_CAUSE_MESSAGE);
--- 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);
+ }
+ }
}
