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

[dmeetry degrave]

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
>