RFR: 8324881: ObjectSynchronizer::inflate(Thread* current...) is invoked for non-current thread [v5]

[David Holmes]

On Fri, 2 Feb 2024 18:26:55 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>>> Given the extra test that @reinrich provided that is exactly what occurs. A nested eliminated lock is re-locked. But the lock is already escaped and contended by another thread.
>> 
>> We didn't reach there with LM_LIGHTWEIGHT. The lock got inflated during warm-up because of the recursive monitorenter in the interpreter. The owner was set to the current thread then. I've improved the test to use a new instance after warm-up. If you want you can cherry-pick here: https://github.com/reinrich/jdk/commit/c28181d585e5ceeaa34e7e6afb4311d59cf990b2
>
> C2 can eliminate 3 types of lock/unlock:
> 1. Locked object does not escape compiled code (it can escape after deoptimization when we return into Interpreter). "Stupid" code example I start seeing in recent bugs:
> 
>    synchronized (new Object())
> 
>   
> 2. Nested locks. There is already held outer lock of the same object (or class) in compiled code.
> 
>     synchronized (obj) {
>          // code
>          synchronized (obj) {
>                // code
> 
> 3. Adjacent unlock/lock pairs if C2 determines it is safe (no control branches or safepoint which can deoptimize between them). We call it `coarsened` locks. You should not see such elimination during deoptimization because no safepoint between.
> 
>     synchronized (obj) { // code1 }
>     synchronized (obj) { // code2 }
> 
> will be converted to
> 
>     synchronized (obj) {
>        // code1
>        // code2
>     }
> 
> 
> Whenever we eliminate lock we mark it in debug info as eliminated so that during deoptimization we can re-lock them.
> It safe to do re-locking for nested locks because deoptee thread already holds lock by outer lock.
> It safe to do re-locking for not-escaped object because other threads do not see it until we return into Interpreter.
> 
> At least that it what happens during normal execution without JVMTI involvement.

> I thought one of the invariants of lightweight locking was that a transition from anonymously owned to actual owner, can only be performed by the actual owner?

To follow up to my own query, this is not quite the case. We introduced the notion of being anonymously owned because under normal conditions if a thread B tries to lock a monitor fast-locked by thread A, it has to inflate the monitor, but it does not know who the owning thread is. Hence it sets it to anonymous. Then when thread A goes to release the monitor, it seems it is inflated, and anonymously owned and so sets the owner to itself, then unlocks.

So as long as we know who the owner thread is, then it should be safe to update the owner from being anonymous.

Also thanks for clarifying the nested lock elimination case, which can lead to the inflation case.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17626#discussion_r1477557201