Hi,
On 2015-11-12 19:10, Paul Sandoz wrote:
Hi Peter,
This stuff always gives me a headache :-)
Yes. :-)
IIUC it’s all idempotent stuff, and the final field in NamedFunction should
take care of certain things.
That's been my understanding, as well.
Claes, was it intentional that you call function.resolve() after the array
store? You might need to reverse that and place a Unsafe.storeFence between
them if it is required that the published and visible function be resolved.
This however was unintentional, and I think you're right that this would
ensure visibility:
function.resolve();
UNSAFE.storeFence();
FUNCTIONS[idx] = function;
http://cr.openjdk.java.net/~redestad/8142334/webrev.04/
/Claes
Paul.
>On 12 Nov 2015, at 17:26, Peter Levart<peter.lev...@gmail.com> 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.
>
>Regards, Peter
