[7u backport] RFR: 7122142: (ann) Race condition between isAnnotationPresent and getAnnotations

Joel Borggren-Franck joel.franck at oracle.com
Thu Feb 27 13:44:11 UTC 2014


Hi,

I looked at webrev.1. Looks good.

cheers
/Joel

On 2014-02-25, dmeetry degrave wrote:
> Thanks for looking at this, Peter!
> 
> On 02/24/2014 04:42 PM, Peter Levart wrote:
> >Hi Dmeetry,
> >
> >On 02/22/2014 01:22 PM, dmeetry degrave wrote:
> >>Hi all,
> >>
> >>I would like to ask for a review of combined back port for
> >>7u-dev/7u80. The main goal is to have a fix for 7122142 in jdk7, it
> >>also integrates the changes from 8005232, 7185456, 8022721
> >>
> >>https://bugs.openjdk.java.net/browse/JDK-7122142
> >>https://bugs.openjdk.java.net/browse/JDK-8005232
> >>https://bugs.openjdk.java.net/browse/JDK-7185456
> >>https://bugs.openjdk.java.net/browse/JDK-8022721
> >>
> >>Original jdk8 changes:
> >>
> >>7122142: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e4ce6502eac0
> >>8005232: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1109bfff4e92
> >>7185456: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ae03282ba501
> >>8022721: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2281a7f79738
> >>
> >>back port:
> >>
> >>http://cr.openjdk.java.net/~dmeetry/7122142.8005232.7185456.8022721/webrev.0/
> >>
> >>
> >>Patches can't be applied cleanly, hence it was a manual back port,
> >>though the final result is equivalent to applying the patches in
> >>chronological order (8005232, 7185456, 7122142, 8022721) and applying
> >>all the relevant rejected parts
> >
> >It's good to see those patches being back-ported to 7u. By browsing the
> >webrev, I don't see any obvious difference between the original patches
> >and the backport.
> 
> there shouldn't be any!
> 
> >Do you happen to remember in what part of code there
> >were rejects so that you had to manually apply the changes?
> 
> there were conflicts due to small difference between 7 and 8
> (copyrights, white spaces, @SuppressWarnings, Class<?>,...).
> 
> I copied all rejected parts and original patches here:
> 
> http://cr.openjdk.java.net/~dmeetry/7122142.8005232.7185456.8022721/webrev.1/rej/
> 
> >>(with one exception, AnnotationTypeRuntimeAssumptionTest.java test was
> >>not included due to jdk8 API).
> >
> >Ah, It's the Class.getDeclaredAnnotation(Class) that's new in JDK8.
> >Here's the changed test that only uses the JDK7 API so you can include
> >this test too:
> >
> >http://cr.openjdk.java.net/~plevart/jdk7u/7122142/AnnotationTypeRuntimeAssumptionTest.java
> 
> Thanks!
> 
> http://cr.openjdk.java.net/~dmeetry/7122142.8005232.7185456.8022721/webrev.1/
> 
> (just with the new test added).
> 
> thanks,
> dmeetry
> >>
> >>All tests in test/java/lang/annotation passed.
> >>
> >>thanks,
> >>dmeetry
> >
> >Regards, Peter
> >



More information about the core-libs-dev mailing list