Am 01.09.2011 22:13, schrieb Sean Mullan:
On 8/31/11 3:26 PM, Sebastian Sickelmann wrote:
Some discussions went on in this thread as i tried to apply this
solution to some other classes.
These classes has adressed the new chaining support in Throwable quite
different.
In the meanwhile i think it is done best the way it is done in
java/lang/{UndeclaredThrowableException|InvocationTargetException|ClassNotFoundException}

I changed the webrev to mirror this pattern of supporting exception
chain from java/lang.
The solution is far easier to understand and i think it needs no
addtional regression-tests.
The solution to delete the cause field and patch serialisation is far
more complex.
I like the deletion of this field much more like prohibiting initCause.
But this change should be mirrored to many Exceptions out there, and
would require many discussion.

Here is the updated webrev:
http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/7011804_0/
Hmm, the main problem I have with this change is that the printStackTrace
methods will no longer print the stack trace of the cause because it will always
be null. That doesn't seem right to me, as it could be considered an
incompatible change, and it will make it harder to debug issues.
The printStackTrace in Throwable calls the overridden getCause().
Maybe we should add @Override to it.
Updated the webrev to:
http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/7011804_1/

IMO, this is one of those bugs where the impact is minimal, and it just might be
better to let things exist as they are, and perhaps add some comments in the
code to that effect.
At least it prevents calls to initCause after creation. Which is now possible.

I additionaly don't like the idea of leaving the cause field in the class but it maybe better to first fix it to an already used pattern (in java/lang) and then start discussion if we should remove the cause field in all implementations where we can remove it without
breaking compatibility.

-- Sebastian
--Sean

Maybe the title of the bug isn't right anymore.
@Alan: Can we change it into something like prohibit unintentional
initCause() after construction?

-- Sebastian
@core-libs-dev: I crosspost it to core-libs-dev because the thread started
there. So the interessted parties can move over to security-dev archive[4]

-- Sebastian

[1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-August/007462.html
[2] 
http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/SeriallizeTest.java
[3] http://oss-patches.24.eu/openjdk8/NoSuchMechanismException/
[4] http://mail.openjdk.java.net/pipermail/security-dev/2011-August/thread.html

Reply via email to