RFR: 8252871: fatal error: must own lock JvmtiThreadState_lock

[David Holmes]

Hi Robbin,

On 8/09/2020 4:56 am, Robbin Ehn wrote:
> When these two methods (set_frame_pop/clear_frame_pop) are called in a handshake the requesting thread will have lock
> the JvmtiThreadState_lock. But the thread executing one of these in the handshake may not be the owner.

Ouch! We continue to get bitten by the fact the executor of a handshake 
operation is not known a-priori. :(

> So we only check that JvmtiThreadState_lock is locked.

That avoids the problem but it is not really a sufficient check - we 
need to know that it is the handshaker thread that owns the lock and 
that it acquired it purely for the purpose of this handshake operation!

More importantly this issue shows that the locking code is in fact 
incorrect. We are on very dangerous ground if we are going to allow 
locks to be "proxied" in this fashion! This is akin to reintroducing 
"sneaky locks"!

If a handshake operation can be executed by either the target thread or 
the handshaker thread, and locks are required, then they will have to be 
acquired in the context of the handshake operation. If we can't do that 
because of potential deadlocks in the handshake mechanism then I feel 
that handshakes have a serious flaw that needs to be rectified. Perhaps 
we need a way to force the operation to be executed by the handshaker so 
that they can setup the right execution environment prior to the handshake?

> When verifying the callers to these methods I notice "clear_to_frame_pop" was unused, so instead of fixing it I remove
> it.

There is already a bug for that:

https://bugs.openjdk.java.net/browse/JDK-8252816

so if this proceeds you should add that issue to the PR.

Thanks,
David
-----

> Passes testing locally, still running T3 and T7.
> 
> Now passed!
> 
> -------------
> 
> Commit messages:
>   - Fix assert and removed unsed methods
> 
> Changes: https://git.openjdk.java.net/jdk/pull/60/files
>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=60&range=00
>    Issue: https://bugs.openjdk.java.net/browse/JDK-8252871
>    Stats: 26 lines in 4 files changed: 0 ins; 24 del; 2 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/60.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/60/head:pull/60
> 
> PR: https://git.openjdk.java.net/jdk/pull/60
>