On Tue, 1 Dec 2020 15:45:45 GMT, Peter Levart <plev...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 1783:
>>
>>> 1781: } finally {
>>> 1782: updateInProgress = false;
>>> 1783: }
>>
>> Line 1782. I understand that in case the try block fails to update `form`
>> field, resetting `updateInProgress` is desirable in order for next caller to
>> attempt to update the form next time, but in case (the majority case) the
>> try block successfully updates the `form` field, resetting the
>> `updateInProgress` back to `false` opens a theoretical window (very
>> improbable) for a concurrent thread to re-execute the updater function at
>> least. Perhaps you could write `catch (Throwable e)` instead of `finally`
>> and only in case of failure, reset the `updateInProgress` field and re-throw
>> the exception. WDYT?
>>
>> It is also very strange that the code is updating a final `form` field using
>> Unsafe. Is this only to ensure that reads of that field scattered around in
>> code are accompanied with a suitable read fence if needed (on some
>> architectures)? How does this work considering (I may be wrong, but I think
>> I heard) that VM trusts final fields in java.lang.invoke package.
>
> It seems that I was right. See `ciField.cpp`:
>
> static bool trust_final_non_static_fields(ciInstanceKlass* holder) {
> if (holder == NULL)
> return false;
> if (holder->name() == ciSymbol::java_lang_System())
> // Never trust strangely unstable finals: System.out, etc.
> return false;
> // Even if general trusting is disabled, trust system-built closures in
> these packages.
> if (holder->is_in_package("java/lang/invoke") ||
> holder->is_in_package("sun/invoke") ||
> holder->is_in_package("jdk/internal/foreign") ||
> holder->is_in_package("jdk/incubator/foreign") ||
> holder->is_in_package("jdk/internal/vm/vector") ||
> holder->is_in_package("jdk/incubator/vector") ||
> holder->is_in_package("java/lang"))
> return true;
I mean, is it possible that some threads that concurrently use the old
uncustomized form while one thread is customizing it, trigger JIT compilation
and because `form` field is trusted final, the JITed code will be using the
uncustomized form for ever. Even after the customization thread plants the new
form... This was possible before, but maybe the probability is greater after
this change.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1472
