RFR: 8062771: Core reflection should use final fields whenever possible

[Joel Borggrén-Franck]

Hi,

I’m having a hard time following this thread, which webrev has been updated, for which release, fixing which issue?

Martin, as far as I can see you are the only one of us who has replied to this thread who is a jdk7u committer (or reviewer).

I think the assumption is that generic repositories of different kinds are ok with multiple initialization, but maybe we need to revisit this?

cheers
/Joel

On 5 nov 2014, at 10:06, Paul Sandoz <paul.sandoz at oracle.com> wrote:

> On Nov 4, 2014, at 6:53 PM, Martin Buchholz <martinrb at google.com> wrote:
>>> Is it possible to make EmptyClass be a static inner class of the test? IIUC if its enclosed within ThreadSafety it should still work with URLClassLoader.
>> 
>> Done.  Webrev regenerated.
> 
> +1
> 
> Perhaps one reason why the test is not failing (on 7) is because certain code is not getting compiled/inlined, since there are not enough iterations. If you think it worth obsessing over you could shuffle the code a bit, increase the iterations of the inner loop, and use some hotspot compiler command options to force inline and/or compile.
> 
> 
>> I recommend y'all do a little correctness backport to jdk7 Class.java
>> as follows:
>> 
>> @@ -2450,7 +2450,7 @@
>>   private native String getGenericSignature();
>> 
>>   // Generic info repository; lazily initialized
>> -    private transient ClassRepository genericInfo;
>> +    private volatile transient ClassRepository genericInfo;
>> 
>>   // accessor for factory
>>   private GenericsFactory getFactory() {
>> @@ -2460,11 +2460,13 @@
>> 
>>   // accessor for generic info repository
>>   private ClassRepository getGenericInfo() {
>> +        ClassRepository genericInfo = this.genericInfo;
>>       // lazily initialize repository if necessary
>>       if (genericInfo == null) {
>>           // create and cache generic info repository
>>           genericInfo = ClassRepository.make(getGenericSignature(),
>>                                              getFactory());
>> +            this.genericInfo = genericInfo;
>>       }
>>       return genericInfo; //return cached repository
>>   }
>> 
> 
> Agreed.
> 
> 
> On Nov 4, 2014, at 9:34 PM, Aleksey Shipilev <aleksey.shipilev at oracle.com> wrote:
>> I think we can make a partial backport of JDK-8016236 to jdk7, since
>> this volatile construction fixes the obvious bug, and the fix was tested
>> already in JDK 8 and 9. Can anyone from Core Libraries team attend to this?
> 
> I am knuckling down into conference development mode this and next week for Devoxx, if no one jumps on it sooner i can sort it out later on.
> 
> Paul.