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
