RFR: 8291555: Implement alternative fast-locking scheme [v56]
Stefan Karlsson
stefank at openjdk.org
Tue Apr 11 14:06:21 UTC 2023
On Tue, 11 Apr 2023 13:48:15 GMT, Roman Kennke <rkennke at openjdk.org> wrote:
>> 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.
>>
>> Did you consider rewriting the racing code to use thread-local handshakes to remove the race?
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1162872406
More information about the serviceability-dev
mailing list