RFR: 8252871: fatal error: must own lock JvmtiThreadState_lock
David Holmes
david.holmes at oracle.com
Tue Sep 8 00:11:30 UTC 2020
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
>
More information about the serviceability-dev
mailing list