RFR: 8291555: Implement alternative fast-locking scheme [v56]
Daniel D. Daugherty
dcubed at openjdk.org
Tue Apr 11 15:33:31 UTC 2023
On Tue, 11 Apr 2023 14:04:17 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> Hmm you are right. But still - that problem is pre-existing, right? Consider this code in the current stack-locking implementation. If we don't stop the other thread, we may end up following a stack-pointer from the locked-object, and by the time we get to that stack-address, the thread may already have given up that lock and the stack-address could contain some other random stuff.
>> Re-writing that code to do a handshake would be nice, but I don't think I want to include this in the scope of this PR. If you agree, I would file a separate issue to investigate the problem. As a band-aid, I could add a 'LockingMode == 2' to the if-statement in management.cpp as I already did it earlier: https://github.com/rkennke/jdk/blob/JDK-8291555-v2/src/hotspot/share/services/management.cpp#L1129. This would make all calls into LockStack::contains() happen at a safepoint or only by self-thread, and would certainly make me sleep a little better ;-)
>
> OK. Given that I haven't looked at the rest of the patch, I leave it up to you and the Reviewers to figure out what to do about this code. Cheers.
Given that the race with new lightweight locking is virtually the same as the race
with legacy stack locking, please do not put back the 'LockingMode == 2' check
which would make `jmm_GetThreadInfo()` calls slower with new lightweight locking
than with legacy stack locking.
Perhaps I'm not understanding the risk of what @stefank means with:
It looks to me like the code could read racingly read the element just above _top,
which could contain a stale oop. If the address of the stale oop matches the
address of o then contains would incorrectly return true.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1162994635
More information about the serviceability-dev
mailing list