RFR: 8325937: runtime/handshake/HandshakeDirectTest.java causes "monitor end should be strictly below the frame pointer" assertion failure on AArch64 [v2]

[Coleen Phillimore]

On Thu, 3 Oct 2024 11:54:53 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Add missing StoreLoad fences in handshaking code to match safepoint code.  Thanks to @pchilano for finding this bug.
>> 
>> Tested with tier1-4 and tier8 which has Kitchensink in it.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove comment about state.

Referring to the comment in the code above, the strange thing is that a whole lot of code isn't broken, which could just be luck.  In the handshake case, we're relying on the fence(), for shared state that's not directly set and accessed inside a Mutex locked critical region.  This case is special and I would like to add the fence() for this case, because we have a specific failure.  Also, it is completely subtle and surprising that this is what we're doing here, and seem like an unnecessary optimization.

We should then file an RFE to determine whether _all_ Mutex operations require an additional fence() so that the Hotspot implementation of Mutex follows it's own stronger specification, OR try to find some proof through assertions that a fence() isn't needed everywhere we take out these locks.  I'm not sure that this is possible but we should do it anyway.  Most of the MutexLockers that I've put in the code is to guard critical section. Maybe that's not prone to these data races.

I can try to capture this conversation in a new bug report.

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

PR Comment: https://git.openjdk.org/jdk/pull/21295#issuecomment-2392096025