RFR: 8142334: Improve lazy initialization of java.lang.invoke

[Claes Redestad]

Hi,

On 2015-11-12 17:26, Peter Levart wrote:
> Hi Claes,
>
> I have one concern...
>
>  645     private static NamedFunction getConstantFunction(int idx) {
>  646         NamedFunction function = FUNCTIONS[idx];
>  647         if (function != null) {
>  648             return function;
>  649         }
>  650         return setCachedFunction(idx, makeConstantFunction(idx));
>  651     }
>  652
>  653     private static synchronized NamedFunction 
> setCachedFunction(int idx, final NamedFunction function) {
>  654         // Simulate a CAS, to avoid racy duplication of results.
>  655         NamedFunction prev = FUNCTIONS[idx];
>  656         if (prev != null) {
>  657             return prev;
>  658         }
>  659         FUNCTIONS[idx] = function;
>  660         function.resolve();
>  661         return function;
>  662     }
>
>
> Above is a classical double-checked locking idiom, but it is not using 
> volatile variable to publish the NamedFunction instance. I wonder if 
> this is safe. Even if the FUNCTIONS[idx] slot was a volatile 
> variable,  you would publish new instance before resolving it. Is it 
> OK to publish unresolved NamedFunction(s)? There is a 
> NamedFunction.resolvedHandle() instance method that makes sure 
> NamedFunction is resolved before returning a MethodHandle, but there 
> are also usages of dereferencing NamedFunction.resolvedHandle field 
> directly in code. Are you sure that such unresolved or almost resolved 
> instance of NamedFunction is never used in such places where 
> NamedFunction.resolvedHandle field is dereferenced directly?
>
> In original code those NamedFunctions were resolved in static 
> initializer so they were published properly.

I think we're relying on the fact that relevant field, member, is final 
and thus will be published correctly.

resolvedHandle will be set to null in the constructor we're using anyhow:

         NamedFunction(Method method) {
             this(new MemberName(method));
         }
...
         NamedFunction(MemberName member) {
             this.member = member;
             this.resolvedHandle = null;
         }

I inquired myself about the rationale for using of DCL without volatile 
elsewhere in the java.lang.invoke code and was pointed to this earlier 
discussion that shed light on the assumptions and caveats: 
http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-May/013902.html

So, all in all, I think the partially unsafe publication is actually 
safe for our purposes here.

Thanks!

/Claes