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
