Request for review Remove "private" cause in jdk exceptions

[Sebastian Sickelmann]

Am 26.08.2011 08:32, schrieb 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).
Sorry for that. It was more a reflex (remove "evil" public fields) than 
a real problem with this. Breaking this would breaking binary 
compatibility but can this be a real show-stopper for not fixing this? 
But you are right. Changes with binary incompatibility should be done 
really carefully.
> 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?
Discussion started at core-libs-dev in another threads[1]. The 
main-reason was to chain more Exceptions together where possible. I 
changed the NoSuchMechanismException in javax.xml.crypto while try to 
chain more Exceptions that extends RuntimeException.
I just removed the field, Alan noted that this would change to 
compatibility and said that this CR should be discussed at security-dev. 
Additionally he created a bug "7081804: Remove cause field from 
javax.xml.crypto.NoSuchMechnismException"[3]

The changes for chained-exceptions in Throwable made to 
javax/xml/crypto/*Exceptions are not done so well as in the other cases 
here. So i tried to fix it like here[2].

Alternatively i could fix javax/xml/crypto by adopting the solution used 
in the other cases.

The question i have is what is the best solution that should used. 
Changing the serialization thought custom serialization or through using 
the solution used in the other cases? I think we should only have one 
solution to address this.
> Cheers,
>
> -- Peter
>
-- Sebastian

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-August/007462.html
[2] 
http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/REBASED_ON_85919cd409d1/
[3] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7081804