RFR: 8065172: More core reflection final and volatile annotations

[Peter Levart]

On 11/24/2014 04:04 PM, Joel Borggrén-Franck wrote:
> Hi,
>
>
>> On 20 Nov 2014, at 16:33, Peter Levart <peter.levart at gmail.com> wrote:
>>
>> Hi Martin,
>>
>> On 11/19/2014 01:42 AM, Martin Buchholz wrote:
>>> Hi Joe, Peter, Paul
>>>
>>> This is the followup on thread safety I promised Peter.
>> Looks good.
> +1
>
>> I made the WildcardTypeImpl.[upperBoundASTs, lowerBoundASTs] and TypeVariableImpl.boundASTs fields volatile in my version of patch (instead of final):
>>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/GenericsReflectionRaces/webrev.01/
>>
>> ...so that after the structure they point to has been parsed into bound types, they can be thrown away. The comments indicate that possibility already, but the implementor was afraid to do it because of possible races. I think I got it right here.
>>
>
> As a cleanup and given the current state of testing here, I slightly favour Martin’s version because it should just make the current behaviour a bit safer.
>
> Btw, has anyone seen the assert for upper/lower bounds == null fail in the wild?
>
> private FieldTypeSignature[] getUpperBoundASTs() {
>      // check that upper bounds were not evaluated yet
>      assert(upperBounds == null);
>      return upperBoundASTs;
> }
>
> shouldn’t it happen once in a while:
>
> public Type[] getUpperBounds() {
>      // lazily initialize bounds if necessary
>      if (upperBounds == null) {
>
> 	// thread gets preempted here, other thread completes init, upperBounds are != null
>
>          FieldTypeSignature[] fts = getUpperBoundASTs(); // get AST
>
>
> Is the current code only working because most run without esa?

...or because races that would trigger this are very rare, since two 
consecutive "loads" of upperBounds happen one after the other, so the 
2nd load and check might even be optimized away.

The private getter for a private field is meaningless in my opinion. 
That's why I removed it in my version of the patch.

Regards, Peter

>
> cheers
> /Joel