RFR: 8225351(13): assert failed: Revoke failed, unhandled biased lock state

[David Holmes]

Hi Robbin,

The other discussion has helped clarify some things but I still have an 
issue with the initial starting premise:

"We may have a stack lock biased against another thread until
ObjectSynchronizer::fast_enter returns."

How do we get a "stack lock" if the object is biased to another thread? 
Doesn't the locking attempt first try to see if the object is already 
biased towards the current thread, and if it finds it is biased towards 
another thread it will first revoke that bias?

That aside, it seems to me that Deoptimization::revoke_using_handshake 
implies that upon return all Monitors found in the frame map are 
guaranteed to be unbiased. But that is not the case as we are only 
revoking biases that are towards the given thread. This breaks the 
symmetry/duality we have with Deoptimization::revoke_using_safepoint. It 
is not clear to me when the bias in that case will actually be revoked.

A minor nit:

1456 void lock_in_top_frame(Handle ho, JavaThread* thread) {

This might read better as check_lock_in_top_frame, else "lock" sounds 
like an action.

Also this file doesn't use the h_x style for naming Handles, so ho could 
just be o, though I'd personally prefer h_o, or better h_obj.

Thanks,
David
-----

On 14/06/2019 1:03 am, Robbin Ehn wrote:
> Hi all, please review.
> 
> When looking at a JavaThreads locks we retrieve them per frame via it's 
> monitors
> list. How this list actually populated differs from frame type. If a 
> JavaThread tries to enter a new monitor a basic lock is 
> directly/indirectly via e.g. scope info added to the virtual monitor 
> list. If this lock is biased towards another
> JavaThread we try to revoke that bias with a safepoint. In this case a 
> deopt
> handshake is already in queue. The handshake is thus executed before the 
> revoke
> safepoint.
> The handshake goes over the monitors in compiled frames, find this lock 
> and we
> hit the assert. The assert make sure we actual can revoke the lock. A 
> basic lock
> on stack should always, if biased, be biased to current thread, with the
> exception:
> We may have a stack lock biased against another thread until
> ObjectSynchronizer::fast_enter returns.
> 
> To handle this exception we can safely ignore biased lock towards other 
> threads
> in the deopt handshake. Since such locks will always be revoked before we
> deopt/unpack stack.
> 
> Code:
> http://cr.openjdk.java.net/~rehn/8225351/v1/webrev/index.html
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8225351
> 
> Passes t1-7
> The assert code tested with local code changes to HandshakeAlot handshake.
> We then see this state where last lock can be biased towards another 
> thread and the thread is trying to execute revoke safepoint.
> 
> Thanks, Robbin