RFR: 8340812: LambdaForm customization via MethodHandle::updateForm is not thread safe

[Jorn Vernee]

On Sun, 29 Sep 2024 23:52:29 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> The `form` field is a final field; thus, all reads to that field is a load-acquire. This load-load barrier is critical to ensure observing the correct, non-null `vmentry` field value if the updated form is observed.
>> 
>> Note that JIT compilation may constant-fold the preexisting form and ignore the updated form. This is still behaviorally identical, but usually the new form is more suitable for constant folding if JIT compilation can pick it up.
>
> That's not exactly how final fields are specified. It is hard to see here exactly what fields/objects are involved in this case. The final field semantics only provide load-acquire-like semantics if you dereference the final field after the freeze action in the constructor. And our use of Unsafe here means we are stepping outside any JLS guarantees anyway.

In this case there is no classic release/acquire. Instead, we rely on other properties: The `vmentry` field is loaded in `MethodHandles::jump_to_lambda_form`, using a plain load. I believe a load-acquire is not needed here because we have a data dependency: we load the `vmentry` field from `form`, and the initialization of the `vmentry` field takes place before the new value of `form` is published. This, combined with the guarantee that a newly allocated object will always have to be freshly loaded after its publication, means that if a thread sees the new `form` field value, it will also see the new `vmentry` field in the subsequent load. (This is also why safe publication works AFAIK)

> A full-fence is a one-sided global synchronization action.

FWIW, a full fence is just a release+acquire on the executing thread. It has no effect on other threads. All the fences in `Unsafe` are one-sided synchronization actions. (The only exception I know of is what is done by `SystemMemoryBarrier` in hotspot)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21160#discussion_r1781477862