JDK-8191785 : revoke_bias() needs a new assertion
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 10
  • Priority: P4
  • Status: Closed
  • Resolution: Won't Fix
  • OS: generic
  • CPU: generic
  • Submitted: 2017-11-22
  • Updated: 2019-08-15
  • Resolved: 2019-05-21
Related Reports
Relates :  
Relates :  
Description
A comment from Robin W's code review on JDK-8167108:

> src/hotspot/share/runtime/biasedLocking.cpp
>
> 217     for (JavaThread* cur_thread = Threads::first(); cur_thread != NULL; cur_thread = cur_thread->next()) {
> 218       if (cur_thread == biased_thread) {
> 219         thread_is_alive = true;
>
> This loop could be replaced with:
>
> ThreadsListHandle tlh;
> thread_is_alive = tlh.includes(biased_thread);

Nice catch! Fixed with your proposed change.

The careful reader will notice that in revoke_bias() we are not
creating a ThreadsListHandle that is in scope for the entire
function and we are doing cached monitor info walks without an
obvious ThreadsListHandle in place.

revoke_bias() is called at a safepoint from most locations. The
one call that is not made at a safepoint is from
BiasedLocking::revoke_and_rebias() and it is made by a JavaThread
that is revoking a bias on itself which is also safe.

We should add an assertion to revoke_bias() that looks something
like this:

  assert(requesting_thread == Thread::current() ||
         SafepointSynchronize::is_at_safepoint(),
         "must be operating on self or at a safepoint.");

but we'll do that in a separate follow up bug since that will
require biased locking focused testing. 
Comments
The biased locking code is changing with JDK-8191890. The assert will be added with that patch to be assert(SafepointSynchronize::is_at_safepoint(), "must be done at safepoint"); since now with handshakes that method will only be called if we are already at a safepoint.
21-05-2019