Request for review Remove "private" cause in jdk exceptions

[Peter Jones]

On Aug 25, 2011, at 8:00 PM, Sebastian Sickelmann wrote:
> Am 26.08.2011 00:24, schrieb Sebastian Sickelmann:
>> Am 26.08.2011 00:03, schrieb Sebastian Sickelmann:
>>> I have found more places in jdk source where an Exception has a private cause field.
>>> 
>>> share/classes/javax/management/remote/JMXProviderException.java:    private Throwable cause = null;
>>> share/classes/javax/xml/crypto/KeySelectorException.java:    private Throwable cause;
>>> share/classes/javax/xml/crypto/NoSuchMechanismException.java:    private Throwable cause;
>>> share/classes/javax/xml/crypto/MarshalException.java:    private Throwable cause;
>>> share/classes/javax/xml/crypto/dsig/XMLSignatureException.java:    private Throwable cause;
>>> share/classes/javax/xml/crypto/dsig/TransformException.java:    private Throwable cause;
>>> share/classes/javax/xml/crypto/URIReferenceException.java:    private Throwable cause;
>>> 
>>> 7081804 handles NoSuchMechanismException.
>>> Is there a way to expand it to at least the xml/crypto/**/* Exceptions?
>>> JMXProviderException should be fine too.
>>> 
>>> I would create a CR for the changes to me made to change and test this.
>>> Would it be good to have some utility-code in Throwable to don't introduce to much code-duplication?
>>> 
>>> -- Sebastian
>> After a very quick analysis i think i found more candidates for removing private causes.
>> share/classes/javax/security/sasl/SaslException.java:    private Throwable _exception;
>> share/classes/java/lang/reflect/UndeclaredThrowableException.java:    private Throwable undeclaredThrowable;
>> share/classes/java/lang/reflect/InvocationTargetException.java:    private Throwable target;
>> share/classes/java/lang/ClassNotFoundException.java:    private Throwable ex;
>> share/classes/com/sun/java/browser/dom/DOMAccessException.java:    private Throwable ex;
>> share/classes/com/sun/java/browser/dom/DOMUnsupportedException.java:    private Throwable ex;
>> share/classes/javax/naming/NamingException.java:    protected Throwable rootException = null;
>> share/classes/java/rmi/RemoteException.java:    public Throwable detail;
>> share/classes/java/rmi/activation/ActivationException.java:    public Throwable detail;
>> 
>> Some of them need deeper inspection. Some of them are the same as the above noted Exceptions in xml/crypto package.
>> 
>> - Sebastian
> OK. Webrev is there: http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/REBASED_ON_8018d541a7b2_2/
> 
> Can someone review this?

Sebastian,

Public fields like RemoteException.detail, ill-advised as they may have been, cannot be removed (would break binary compatibility).

This change proposes additions to the public API (subclass interface) of java.lang.Throwable, which would need additional approval process.  Without getting into detailed design critique, the proposed protected methods seem to add subtle complexity to the subclass interface of this central class, which leads me to:

Is there a particular problem that these changes are attempting to address?  Many of these exception classes had cause-like fields prior to the addition of Throwable.cause in 1.4, and as described in their class docs, for 1.4 they were each retrofitted, with some care, to work well with the general cause APIs added to Throwable.  (Also see the doc for Throwable.getCause, which describes how subclasses can override it to take responsibility for their causes.)

These changes seem to be about implementing an alternate approach to that retrofitting, with considerably higher complexity (serialized form compatibility code) and risk, and I don't understand why-- I don't see a motivation discussed earlier on core-libs-dev?

Cheers,

-- Peter