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

[Jorn Vernee]

On Tue, 1 Oct 2024 06:28:02 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> 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 like that. (The only exception I know of is what is done by `SystemMemoryBarrier` in hotspot)
>
>> In this case there is no classic release/acquire. Instead, we rely on other properties:
> 
> Relying on data-dependency is making assumptions about what the underlying hardware memory models actually do, and is something we should generally avoid in shared code. In the abstract/general sense without a matching explicit acquire action, nothing guarantees you will see writes prior to a release-store. If we are going to rely on data-dependency then it should be clearly documented. See discussion/definitions in src/hotspot/share/runtime/orderAccess.hpp
> 
>> FWIW, a full fence is just a release+acquire on the executing thread. It has no effect on other threads. 
> 
> A full fence is more than just release+acquire as it also implies a storeLoad barrier. A full fence should ensure consistent ordering/visibility across all threads (store barriers on sparc, mfence (or equivalent) on x86, DMB on ARM,  sync on PPC). At least that is the expected semantics for hotspot.

Ok, I was under the, apparently false, impression that synchronization _always_ required some kind of action by both threads (I'm more likely to be misremembering though). But from reading the section on cache coherency in orderAccess.hpp, it sounds like these fences are at least semantically expected to affect other threads.

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

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