Bug ID: JDK-7005628 Clarify NPE behavior of Throwable.addSuppressed(null)

Type: Enhancement
Component: core-libs
Sub-Component: java.lang
Affected Version: 7

Priority: P4
Status: Closed
Resolution: Fixed
OS: generic
CPU: generic

Submitted: 2010-12-09
Updated: 2017-05-16
Resolved: 2011-04-27

JDK 7
7 b138Fixed

The JCK team has requested a specification clarification over whether the second call to addSuppressed below should trigger an NPE

Throwable t = new Throwable();
t.addSuppressed(null); // Okay
t.addSuppressed(null); // Currently triggers an NPE

SUGGESTED FIX # HG changeset patch # User darcy # Date 1301623742 25200 # Node ID 856cc9e97aeac8ca2786fbec5e789548aa99c556 # Parent 683957148bab1d2d5476017e22d7773d3cf7bf45 7005628: Clarify NPE behavior of Throwable.addSuppressed(null) Reviewed-by: dholmes, mchung, jjb --- a/src/share/classes/java/lang/ArithmeticException.java Thu Mar 31 17:37:11 2011 +0100 +++ b/src/share/classes/java/lang/ArithmeticException.java Thu Mar 31 19:09:02 2011 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1994, 2008, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1994, 2011, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -30,15 +30,18 @@ package java.lang; * example, an integer "divide by zero" throws an * instance of this class. * + * {@code ArithmeticException} objects may be constructed by the + * virtual machine as if {@linkplain Throwable#Throwable(String, + * Throwable, boolean) suppression were disabled}. + * * @author unascribed * @since JDK1.0 */ -public -class ArithmeticException extends RuntimeException { +public class ArithmeticException extends RuntimeException { private static final long serialVersionUID = 2256477558314496007L; /** - * Constructs an <code>ArithmeticException</code> with no detail + * Constructs an {@code ArithmeticException} with no detail * message. */ public ArithmeticException() { @@ -46,7 +49,7 @@ class ArithmeticException extends Runtim } /** - * Constructs an <code>ArithmeticException</code> with the specified + * Constructs an {@code ArithmeticException} with the specified * detail message. * * @param s the detail message. --- a/src/share/classes/java/lang/NullPointerException.java Thu Mar 31 17:37:11 2011 +0100 +++ b/src/share/classes/java/lang/NullPointerException.java Thu Mar 31 19:09:02 2011 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1994, 2008, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1994, 2011, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,20 +26,24 @@ package java.lang; package java.lang; /** - * Thrown when an application attempts to use <code>null</code> in a + * Thrown when an application attempts to use {@code null} in a * case where an object is required. These include: * <ul> - * <li>Calling the instance method of a <code>null</code> object. - * <li>Accessing or modifying the field of a <code>null</code> object. - * <li>Taking the length of <code>null</code> as if it were an array. - * <li>Accessing or modifying the slots of <code>null</code> as if it + * <li>Calling the instance method of a {@code null} object. + * <li>Accessing or modifying the field of a {@code null} object. + * <li>Taking the length of {@code null} as if it were an array. + * <li>Accessing or modifying the slots of {@code null} as if it * were an array. - * <li>Throwing <code>null</code> as if it were a <code>Throwable</code> + * <li>Throwing {@code null} as if it were a {@code Throwable} * value. * </ul> * * Applications should throw instances of this class to indicate - * other illegal uses of the <code>null</code> object. + * other illegal uses of the {@code null} object. + * + * {@code NullPointerException} objects may be constructed by the + * virtual machine as if {@linkplain Throwable#Throwable(String, + * Throwable, boolean) suppression were disabled}. * * @author unascribed * @since JDK1.0 @@ -49,14 +53,14 @@ class NullPointerException extends Runti private static final long serialVersionUID = 5162710183389028792L; /** - * Constructs a <code>NullPointerException</code> with no detail message. + * Constructs a {@code NullPointerException} with no detail message. */ public NullPointerException() { super(); } /** - * Constructs a <code>NullPointerException</code> with the specified + * Constructs a {@code NullPointerException} with the specified * detail message. * * @param s the detail message. --- a/src/share/classes/java/lang/OutOfMemoryError.java Thu Mar 31 17:37:11 2011 +0100 +++ b/src/share/classes/java/lang/OutOfMemoryError.java Thu Mar 31 19:09:02 2011 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1994, 2008, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1994, 2011, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -30,22 +30,25 @@ package java.lang; * because it is out of memory, and no more memory could be made * available by the garbage collector. * + * {@code OutOfMemoryError} objects may be constructed by the virtual + * machine as if {@linkplain Throwable#Throwable(String, Throwable, + * boolean) suppression were disabled}. + * * @author unascribed * @since JDK1.0 */ -public -class OutOfMemoryError extends VirtualMachineError { +public class OutOfMemoryError extends VirtualMachineError { private static final long serialVersionUID = 8228564086184010517L; /** - * Constructs an <code>OutOfMemoryError</code> with no detail message. + * Constructs an {@code OutOfMemoryError} with no detail message. */ public OutOfMemoryError() { super(); } /** - * Constructs an <code>OutOfMemoryError</code> with the specified + * Constructs an {@code OutOfMemoryError} with the specified * detail message. * * @param s the detail message. --- a/src/share/classes/java/lang/Throwable.java Thu Mar 31 17:37:11 2011 +0100 +++ b/src/share/classes/java/lang/Throwable.java Thu Mar 31 19:09:02 2011 -0700 @@ -52,7 +52,7 @@ import java.util.*; * throwable can {@linkplain Throwable#addSuppressed suppress} other * throwables from being propagated. Finally, the throwable can also * contain a cause: another throwable that caused this - * throwable to get thrown. The recording of this causal information + * throwable to be constructed. The recording of this causal information * is referred to as the chained exception facility, as the * cause can, itself, have a cause, and so on, leading to a "chain" of * exceptions, each caused by another. @@ -280,6 +280,41 @@ public class Throwable implements Serial fillInStackTrace(); detailMessage = (cause==null ? null : cause.toString()); this.cause = cause; + } + + /** + * Constructs a new throwable with the specified detail message, + * cause, and {@linkplain #addSuppressed suppression} enabled or + * disabled. If suppression is disabled, {@link #getSuppressed} + * for this object will return a zero-length array and calls to + * {@link #addSuppressed} that would otherwise append an exception + * to the suppressed list will have no effect. + * + * Note that the other constructors of {@code Throwable} treat + * suppression as being enabled. Subclasses of {@code Throwable} + * should document any conditions under which suppression is + * disabled. Disabling of suppression should only occur in + * exceptional circumstances where special requirements exist, + * such as a virtual machine reusing exception objects under + * low-memory situations. + * + * @param message the detail message. + * @param cause the cause. (A {@code null} value is permitted, + * and indicates that the cause is nonexistent or unknown.) + * @param enableSuppression whether or not suppression is enabled or disabled + * + * @see OutOfMemoryError + * @see NullPointerException + * @see ArithmeticException + * @since 1.7 + */ + protected Throwable(String message, Throwable cause, + boolean enableSuppression) { + fillInStackTrace(); + detailMessage = message; + this.cause = cause; + if (!enableSuppression) + suppressedExceptions = null; } /** @@ -830,13 +865,10 @@ public class Throwable implements Serial * typically called (automatically and implicitly) by the {@code * try}-with-resources statement. * - * If the first exception to be suppressed is {@code null}, that - * indicates suppressed exception information will not be - * recorded for this exception. Subsequent calls to this method - * will not record any suppressed exceptions. Otherwise, - * attempting to suppress {@code null} after an exception has - * already been successfully suppressed results in a {@code - * NullPointerException}. + * The suppression behavior is enabled unless disabled + * {@linkplain #Throwable(String, Throwable, boolean) via a + * constructor}. When suppression is disabled, this method does + * nothing other than to validate its argument. * * Note that when one exception {@linkplain * #initCause(Throwable) causes} another exception, the first @@ -874,33 +906,23 @@ public class Throwable implements Serial * suppressed exceptions * @throws IllegalArgumentException if {@code exception} is this * throwable; a throwable cannot suppress itself. - * @throws NullPointerException if {@code exception} is null and - * an exception has already been suppressed by this exception + * @throws NullPointerException if {@code exception} is {@code null} * @since 1.7 */ public final synchronized void addSuppressed(Throwable exception) { if (exception == this) throw new IllegalArgumentException(SELF_SUPPRESSION_MESSAGE); - if (exception == null) { - if (suppressedExceptions == SUPPRESSED_SENTINEL) { - suppressedExceptions = null; // No suppression information recorded - return; - } else - throw new NullPointerException(NULL_CAUSE_MESSAGE); - } else { - assert exception != null && exception != this; - - if (suppressedExceptions == null) // Suppressed exceptions not recorded - return; - - if (suppressedExceptions == SUPPRESSED_SENTINEL) - suppressedExceptions = new ArrayList<>(1); - - assert suppressedExceptions != SUPPRESSED_SENTINEL; - - suppressedExceptions.add(exception); - } + if (exception == null) + throw new NullPointerException(NULL_CAUSE_MESSAGE); + + if (suppressedExceptions == null) // Suppressed exceptions not recorded + return; + + if (suppressedExceptions == SUPPRESSED_SENTINEL) + suppressedExceptions = new ArrayList<>(1); + + suppressedExceptions.add(exception); } private static final Throwable[] EMPTY_THROWABLE_ARRAY = new Throwable[0]; @@ -910,7 +932,9 @@ public class Throwable implements Serial * suppressed, typically by the {@code try}-with-resources * statement, in order to deliver this exception. * - * If no exceptions were suppressed, an empty array is returned. + * If no exceptions were suppressed or {@linkplain + * Throwable(String, Throwable, boolean) suppression is disabled}, + * an empty array is returned. * * @return an array containing all of the exceptions that were * suppressed to deliver this exception. --- a/test/java/lang/Throwable/SuppressedExceptions.java Thu Mar 31 17:37:11 2011 +0100 +++ b/test/java/lang/Throwable/SuppressedExceptions.java Thu Mar 31 19:09:02 2011 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,7 +26,7 @@ import java.util.*; /* * @test - * @bug 6911258 6962571 6963622 6991528 + * @bug 6911258 6962571 6963622 6991528 7005628 * @summary Basic tests of suppressed exceptions * @author Joseph D. Darcy */ @@ -44,14 +44,6 @@ public class SuppressedExceptions { private static void noSelfSuppression() { Throwable throwable = new Throwable(); - try { - throwable.addSuppressed(throwable); - throw new RuntimeException("IllegalArgumentException for self-suppresion not thrown."); - } catch (IllegalArgumentException iae) { - ; // Expected - } - - throwable.addSuppressed(null); // Immutable suppression list try { throwable.addSuppressed(throwable); throw new RuntimeException("IllegalArgumentException for self-suppresion not thrown."); @@ -153,19 +145,19 @@ public class SuppressedExceptions { (byte)0x02, (byte)0x00, (byte)0x00, (byte)0x78, (byte)0x70, }; - ByteArrayInputStream bais = new ByteArrayInputStream(bytes); - ObjectInputStream ois = new ObjectInputStream(bais); - - Object o = ois.readObject(); - Throwable throwable = (Throwable) o; - - System.err.println("TESTING SERIALIZED EXCEPTION"); - - Throwable[] t0 = throwable.getSuppressed(); - if (t0.length != 0) { // Will fail if t0 is null. - throw new RuntimeException(message); - } - throwable.printStackTrace(); + try(ByteArrayInputStream bais = new ByteArrayInputStream(bytes); + ObjectInputStream ois = new ObjectInputStream(bais)) { + Object o = ois.readObject(); + Throwable throwable = (Throwable) o; + + System.err.println("TESTING SERIALIZED EXCEPTION"); + + Throwable[] t0 = throwable.getSuppressed(); + if (t0.length != 0) { // Will fail if t0 is null. + throw new RuntimeException(message); + } + throwable.printStackTrace(); + } } private static void selfReference() { @@ -183,8 +175,7 @@ public class SuppressedExceptions { } private static void noModification() { - Throwable t = new Throwable(); - t.addSuppressed(null); + Throwable t = new NoSuppression(false); Throwable[] t0 = t.getSuppressed(); if (t0.length != 0) @@ -196,5 +187,24 @@ public class SuppressedExceptions { t0 = t.getSuppressed(); if (t0.length != 0) throw new RuntimeException("Bad nonzero length of suppressed exceptions."); + + Throwable suppressed = new ArithmeticException(); + t = new NoSuppression(true); // Suppression enabled + // Make sure addSuppressed(null) throws an NPE + try { + t.addSuppressed(null); + } catch(NullPointerException e) { + ; // Expected + } + t.addSuppressed(suppressed); + t0 = t.getSuppressed(); + if (t0.length != 1 || t0[0] != suppressed) + throw new RuntimeException("Expected suppression did not occur."); + } + + private static class NoSuppression extends Throwable { + public NoSuppression(boolean enableSuppression) { + super("The medium.", null, enableSuppression); + } } }

01-04-2011

PUBLIC COMMENTS See http://hg.openjdk.java.net/jdk7/tl/jdk/rev/856cc9e97aea

01-04-2011

EVALUATION A fine idea.

09-12-2010

Relates :	JDK-4988583 - AutomaticException chaining for exception in try and exception thrown in finally
Relates :	JDK-6978918 - Should clear stackTrace before throwing the preallocated default OutOfMemoryError objects