7122142 : (ann) Race condition between isAnnotationPresent and getAnnotations
Joel Borggrén-Franck
joel.franck at oracle.com
Wed Jul 3 17:09:49 UTC 2013
Hi Peter,
As Alan said, a big thanks for looking into this.
I have been toying with a slightly different approach to breaking the infinite recursion by pre-construct AnnotationType instances for Retention and Inherited. While cleaner in some places others became uglier. I'm fine with this approach.
As Alan said, you need to update the patch with a modification in TypeAnnotationParser. After doing that all java/lang tests in the jdk/test directory passes.
Also, can you please explain to me why the update race is safe. I have done the/some research myself but I would like to know which angles you have covered.
Given that and that you fix Alan's comments I think this is good to go.
Thanks again!
cheers
/Joel
On Jun 24, 2013, at 8:23 PM, Peter Levart <peter.levart at gmail.com> wrote:
> On 06/19/2013 08:54 PM, Alan Bateman wrote:
>> Thank you for coming back to this.
>>
>> I've looked over the webrev and the approach looks good to me. Joel might want to look at this too. Do you think you could include a test (as we try to include a test with all fixes if we can)?
>>
>> It would be good to remove the synchronizaiton on initAnnotationsIfNecessary too, but one step as time (and smaller changes are always easier to discuss).
>>
>> -Alan
>
> Hi Alan,
>
> I have prepared the 2nd revision of the patch:
>
> http://cr.openjdk.java.net/~plevart/jdk8-tl/AnnotationType/webrev.02/
>
> There is a little change in AnnotationParser's alternative parsing method. This time the parser does not assume anything about annotations being parsed (previously it assumed that they had RUNTIME retention). The only difference from standard parsing is that only the select annotation types are parsed and the rest are quickly skipped. Infinite recursion is broken by the special cased evaluation in AnnotationType constructor:
>
> 129 // Initialize retention, & inherited fields. Special treatment
> 130 // of the corresponding annotation types breaks infinite recursion.
> 131 if (annotationClass != Retention.class &&
> 132 annotationClass != Inherited.class) {
> 133 JavaLangAccess jla = sun.misc.SharedSecrets.getJavaLangAccess();
> 134 Map<Class<? extends Annotation>, Annotation> metaAnnotations =
> 135 AnnotationParser.parseAnnotations(
> 136 jla.getRawClassAnnotations(annotationClass),
> 137 jla.getConstantPool(annotationClass),
> 138 annotationClass,
> 139 Retention.class, Inherited.class
> 140 );
> 141 Retention ret = (Retention) metaAnnotations.get(Retention.class);
> 142 retention = (ret == null ? RetentionPolicy.CLASS : ret.value());
> 143 inherited = metaAnnotations.containsKey(Inherited.class);
> 144 }
> 145 else {
> 146 retention = RetentionPolicy.RUNTIME;
> 147 inherited = false;
> 148 }
>
>
> This is enough to break recursion. The RUNTIME and !inherited assumptions for @Retention and @Inherited meta-annotations are therefore localized in this code only.
>
> I have also added two tests. The one for detecting deadlock situation and the other for consistent parsing of mutually recursive annotations in presence of separate compilation.
>
>
>
> Regards, Peter
>
More information about the core-libs-dev
mailing list