On Tue, 11 Apr 2023 15:29:16 GMT, Daniel D. Daugherty <dcu...@openjdk.org> 
wrote:

>> 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.

The `_base` array is only initialized to nullptr in debug builds.  I don't see 
a release barrier in LockStack::push between the update to _base[] and the 
update to _top, nor a corresponding acquire barrier when reading.  Doesn't this 
mean it is possible to racily read an uninitialized junk oop value from 
_base[], especially on weak memory models?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1163243827

Reply via email to