RFR: 8254263: Remove special_runtime_exit_condition() check from ~ThreadInVMForHandshake()

David Holmes dholmes at openjdk.java.net
Sun Oct 11 22:23:09 UTC 2020


On Fri, 9 Oct 2020 14:10:16 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

> Hi all,
> 
> Please review the following patch that removes the call to handle_special_runtime_exit_condition() in
> ~ThreadInVMForHandshake(). This check was added by 8223572 to prevent a JavaThread that was suspended while trying to
> process a handshake to transition back to java without noticing it was suspended. Since 8238761, a JavaThread executing
> HandshakeState::process_by_self() will never become safe. It comes from an unsafe state and remains unsafe, so it
> cannot be suspended during that time. Removing this check also removes one of the reasons
> SafepointMechanism::process_if_requested() is recursive (the other one remains SafepointSynchronize::block()). Tested
> in mach5, tiers 1-7.  Thanks, Patricio

Just to clarify my own understanding. Looking at the original problem scenario in JDK-8223572 we have:
- T calls _semaphore.wait_with_safepoint_check()
- T transitions from _thread_in_vm to _thread_blocked
- The VM thread completes H and calls _semaphore.signal()
- Before T can transition back to _thread_in_vm, the VM thread schedules another safepoint and
  executes VM_ThreadSuspend on behalf of a JVMTI agent that wants to suspend T

The key change in JDK-8238761 is that we now call:

` MutexLocker ml(&_lock, Mutex::_no_safepoint_check_flag);`

so there is longer the possibility of T becoming handshake-safe and so the VM_ThreadSuspend cannot happen.

src/hotspot/share/runtime/interfaceSupport.inline.hpp line 146:

> 144:
> 145:   ~ThreadInVMForHandshake() {
> 146:     assert(_thread->thread_state() == _thread_in_vm, "should only call when leaving VM after handshake");

Can we also assert `!_thread->has_special_runtime_exit_condition()`? Or at least that the value of this has not changed
between the TIVFH constructor and destructor.

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

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/577


More information about the hotspot-runtime-dev mailing list