RFR: 8341819: LightweightSynchronizer::enter_for races with deflation

[Patricio Chilano Mateo]

On Wed, 9 Oct 2024 11:16:46 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

> This is a regression from [JDK-8315884](https://bugs.openjdk.org/browse/JDK-8315884).
> 
> When using `+UseObjectMonitorTable` monitors are inflated in a locked state effectively blocking out deflation. `LightweightSynchronizer::enter_for` assumed this to be true. But when the `-UseObjectMonitorTable` path was added `// Do the old inflate and enter.` this is no longer true as it first inflates a monitor in an unlocked state and then tries to lock. We need to introduce a retry loop similar to what was used before [JDK-8315884](https://bugs.openjdk.org/browse/JDK-8315884).
> 
> I propose we add this retry loop for both cases, to decouple the `LightweightSynchronizer::enter_for` from how lock elimination is done. With a retry loop, the only requirements for using `LightweightSynchronizer::enter_for` is that the Object locked on cannot have been locked on by another thread, i.e. there is no contention, but makes no assumptions on the interaction with the deflation thread.
> 
> For `-UseObjectMonitorTable` 7bdbe114eb57fe7310f9664a434c4d9203e656fc should be enough, as it will assist the deflating thread with deflation, so the second call must succeed. 
> 
> However `+UseObjectMonitorTable` cannot do this so it must wait for the deflating thread to make progress. But as mentioned above, this would only happen if partial lock elimination is performed. E.g.
> 
> Object o = new Object();
> synchronized(o) {
>   o.wait(1);
> }
> synchronized(o) {
>   deoptimize();
> }
> 
> got transformed to 
> 
> Object o = new Object();
> synchronized(o) {
>   o.wait(1);
> }
> // synchronized(o) { Eliminated lock
>   deoptimize();
> // }
> 
> 
> As far as I can tell, this does not happen. But I do not want to couple lock elimination decision with `LightweightSynchronizer::enter_for`. So I propose a retry loop instead of just the two calls.
> After this change the only prerequisite for using `LightweightSynchronizer::enter_for` is that the object being synchronized can not have been reached by another JavaThread (except the deflating thread).  So there may never be contention, but there may be deflation.

Looks good to me, thanks.

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

Marked as reviewed by pchilanomate (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21420#pullrequestreview-2357290865