Re: Request for approval for bug #8031488
On 10/01/2014 16:28, Iaroslav Savytskyi wrote: : There are 3 possibilities: 1) Because it was my own initiative to fix this potential synchronization bug and nobody didn’t report it, we can approve my fix and leave this 2 classes without synchronized getters. And fix it in MR. 2) Fix it as you propose. But later we will definitely need to change it again to volatile for performance reasons. 3) Leave classes with volatile as they are now. And only add SUID to TypeConstraintException class. I know this is a blocker for JDK 8 and I don't want to waste time debating the options. So I think #1 or #2 are okay (with a slight prefer for #2 because it doesn't require bring back the original bug in JAXBException). #3 is of course the best but from the previous mails then I thought this wasn't an option. So you choose and one of us will help you get this in. -Alan.
Request for approval for bug #8031488
Hello, I would like to request for approval for this fix. This is simple revert of the changes which caused the issue. I’ve returned back synchronization and removed volatile. So now serialVersionUID is the same as before. Bug: https://bugs.openjdk.java.net/browse/JDK-8031488 Webrev: http://cr.openjdk.java.net/~mkos/8027908/webrev.00 Thank you. — Best regardsю Iaroslav
Re: Request for approval for bug #8031488
Hi, Alan, You are absolutely right. Unfortunately the things a little bit more complicated. The reason why I’m fixing this now is, that some time ago I fix this synchronization issue (synchronized setter without synchronized getter). After that I got this bug. We had internal discussions if I can leave my changes and the short answer is “no” :| Because it’s JAXB API and I can’t change signatures within the same version. So I have to revert my changes and leave it as it was before. We will fix this in the next MR for JAXB API. -- Regards. Iaroslav On 10 Jan 2014, at 15:52, Alan Bateman alan.bate...@oracle.com wrote: On 10/01/2014 14:26, Iaroslav Savytskyi wrote: Hello, I would like to request for approval for this fix. This is simple revert of the changes which caused the issue. I’ve returned back synchronization and removed volatile. So now serialVersionUID is the same as before. Bug: https://bugs.openjdk.java.net/browse/JDK-8031488 Webrev: http://cr.openjdk.java.net/~mkos/8027908/webrev.00 If these are changed to use synchronization then maybe you want to change getLinkedException too. In any case, isn't the right thing to just add the serialVersionUID? That is, I assume the issue that the missing serialVersionUID meant the default SUID changed when you changed a field modifier. -Alan.
Re: Request for approval for bug #8031488
On 10/01/2014 15:08, Iaroslav Savytskyi wrote:
Hi, Alan,
You are absolutely right. Unfortunately the things a little bit more
complicated. The reason why I’m fixing this now is, that some time ago I fix
this synchronization issue (synchronized setter without synchronized getter).
After that I got this bug. We had internal discussions if I can leave my
changes and the short answer is “no” :| Because it’s JAXB API and I can’t
change signatures within the same version. So I have to revert my changes and
leave it as it was before. We will fix this in the next MR for JAXB API.
It looks to me that JAXBException has always defined its SUID so I
assume this means there isn't really any need to revert that, right?
For TypeConstraintException then adding the SUID to the value that it
has always been doesn't change anything except that it now appears in
the serialization form that javadoc reports. So any implementation of
this exception type would need to use that value. That might be why you
need to do a MR. If it is required then your change is okay although I
think it would be better if getLinkedException was synchronized. You can
use synchronized (this) { ... } around the read to avoid changing the
modifiers (and hence SUID).
-Alan.
Re: Request for approval for bug #8031488
On 10 Jan 2014, at 16:36, Alan Bateman alan.bate...@oracle.com wrote:
On 10/01/2014 15:08, Iaroslav Savytskyi wrote:
Hi, Alan,
You are absolutely right. Unfortunately the things a little bit more
complicated. The reason why I’m fixing this now is, that some time ago I fix
this synchronization issue (synchronized setter without synchronized
getter). After that I got this bug. We had internal discussions if I can
leave my changes and the short answer is “no” :| Because it’s JAXB API and I
can’t change signatures within the same version. So I have to revert my
changes and leave it as it was before. We will fix this in the next MR for
JAXB API.
It looks to me that JAXBException has always defined its SUID so I assume
this means there isn't really any need to revert that, right?
Yes, JAXBException has SUID. The reason I’ve reverted my changes is that I’m
going to fix both files at once in MR.
For TypeConstraintException then adding the SUID to the value that it has
always been doesn't change anything except that it now appears in the
serialization form that javadoc reports. So any implementation of this
exception type would need to use that value. That might be why you need to do
a MR. If it is required then your change is okay although I think it would be
better if getLinkedException was synchronized. You can use synchronized
(this) { ... } around the read to avoid changing the modifiers (and hence
SUID).
-Alan.
There are 3 possibilities:
1) Because it was my own initiative to fix this potential synchronization bug
and nobody didn’t report it, we can approve my fix and leave this 2 classes
without synchronized getters. And fix it in MR.
2) Fix it as you propose. But later we will definitely need to change it again
to volatile for performance reasons.
3) Leave classes with volatile as they are now. And only add SUID to
TypeConstraintException class.
—
Iaroslav
