Helo,

I need a Reviewer for the following patch:

http://cr.openjdk.java.net/~plevart/jdk8-tl/AnnotationType/webrev.05/

This fixes deadlock situation described in bug:

    http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7122142

The in-depth evaluation of the patch is described in the introductory message of this thread:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018226.html

The above is the 5th revision of the patch that also makes sure a single AnnotationType instance is ever returned for the same Class object even when multiple threads concurrently request one.


Regards, Peter

On 07/05/2013 04:04 PM, Joel Borggrén-Franck wrote:
Hi Peter,

Thanks for the quick update!

While I have looked over the changes to j.l.Class and the cas in AnnotationType 
I don't think I'm qualified to review that. (FWIW it looked fine to me but my 
jmm isn't swapped in at the moment so I won't pretend to know the interactions 
between volatile and Unsafe cas). Thinking out loud I suppose we can assume a 
stable offset of fields in Class, and I realize that part has been under review 
before.

The rest of AnnotationType, AnnotationParser and the tests look fine though. I 
also ran the tests before and after the change and results were as expected.

Since I'm not a Reviewer kind of reviewer you need to get one those to sign off 
but after that I think you are good to go. I can sponsor this as well if Alan 
is busy.

cheers
/Joel

On 5 jul 2013, at 11:12, Peter Levart <peter.lev...@gmail.com> wrote:

Hi Again,

Sorry, the 4th revision I posted is not rebased to the current tip of jdk8-tl 
so it contained some diffs that reverted some things.

Here's the correct patch:

http://cr.openjdk.java.net/~plevart/jdk8-tl/AnnotationType/webrev.05/


Regards, Peter


On 07/05/2013 10:32 AM, Peter Levart wrote:
Hi Joel,

Here's the 4th revision of the patch:

http://cr.openjdk.java.net/~plevart/jdk8-tl/AnnotationType/webrev.04/

This one introduces CAS-ing of the AnnotationType instance into the 
Class.annotationType field so that there's only a single instance ever returned 
for a single Class. I also introduced new private static Class.Atomic nested 
class that is used to lazily initialize Unsafe machinery and to provide a safe 
wrapper for internal j.l.Class use. Previously this was part of ReflectionData 
nested class because it was only used for it's purpose. In anticipation to have 
a need for more atomic operations in the future, I moved this code to a 
separate class. ReflectionData is now just a record.

Regards, Peter

Reply via email to