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

[Patricio Chilano]

Hi Robbin,

Change looks good to me!

One question though. In deoptimization.cpp there is method 
fetch_unroll_info_helper() which calls create_vframeArray() which will 
eventually call vframeArrayElement::fill_in(). That method tries to get 
the monitors from the stack and has the following assertion:

assert(monitor->owner() == NULL || (!monitor->owner()->is_unlocked() && 
!monitor->owner()->has_bias_pattern()), "object must be null or locked, 
and unbiased");

So it is expecting that the locked monitors are not biased anymore. Do 
you know exactly when fetch_unroll_info_helper() would be called during 
this deoptimization phase? There is a comment "fetch_unroll_info() is 
called at the beginning of the deoptimization handler" but no sure what 
that means exactly since it is expecting monitors to be revoked already.
My question is whether we could hit that assert later on if we avoid 
trying to revoke these biased objects that actually belong to somebody 
else. One scenario could be as follows:

1) JT1 calls BiasedLocking::revoke_and_rebias() for object X, finds X is 
biased toward JT2 and requests a revocation using a bulk rebias 
operation. The attempt_rebias flag is true, so this call comes from 
interpreter/c1/c2 where JT1 was trying to lock this object.
2) VMThread executes a deoptimization operation (present in the VM queue 
before the bulk rebias operation was added). VMThread executes the 
handshake on behalf of JT1, who is blocked, finds monitor associated 
with object X in the stack but skips revocation since JT1 is not the 
owner. JT2 executes the handshake too
but does not revoke bias of X because it is not currently using that 
lock (it's not in its stack).
3) VMThread executes the bulk rebias operation for objects of class X 
that JT1 requested. Object  X is not currently in the stack of JT2 and 
so the bias for object X expires and is not valid anymore. Since 
bulk_revoke_or_rebias_at_safepoint() was called with a value of 
attempt_rebias_of_object=true the object will be rebiased towards JT1.
4) JT1 unblocks, and tries to execute fetch_unroll_info_helper() (?)

But not sure if that last step is possible.


Thanks Robbin!


Patricio

On 6/14/19 4: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