RFR: 8274282: Clarify special wait assert [v2]

Coleen Phillimore coleenp at openjdk.java.net
Mon Oct 4 19:37:08 UTC 2021


On Mon, 4 Oct 2021 13:07:43 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> I can probably shed some light on the origin of this comment since it came from discussions I had with Coleen. We were looking at cases like the Service_lock, where the ServiceThread waits on a lock with rank <= nosafepoint but within the context of a ThreadBlockInVM. If a JT would call wait() on a nosafepoint lock and we detect it holds some other nosafepoint lock, then that would imply there was no immediate TBIVM, like with the Service_lock case, because that would have asserted due to the NSV and so we know the call will block safepoints (assuming no manual changes to _thread_blocked that would bypass the check_possible_safepoint() call or some weird use of the TBIVM like: TBIVM, lock(a), lock(b), wait(b), where a and b are nosafepoint locks). But as you pointed out the issue of this JT blocking safepoints is a consequence of waiting on nosafepoint locks, regardless of which other locks the JT is holding. It just happens that if the JT is holding other nosafepoint locks then w
 e already know this will block out safepoints (ruling out those behaviors described above).
>> 
>> I also just realized that waiting while holding a nosafepoint lock could also block out safepoints due to other JT's being blocked in a safepoint-unsafe state while trying to lock that monitor. So even if the waiting JT did a manual transition to _thread_blocked or used the TBIVM in some weird way, safepoints could still be blocked out due to those other JT's.
>> 
>> But in any case, if we want to verify if a wait() call might block safepoints, we should verify all cases where this JT will wait in an unsafe state, and that means checking whether the current lock is nosafepoint and the current state of the JT: "if thread->is_Java_thread() && this->rank <= Mutex::nosafepoint && (thread->thread_state() != _thread_blocked)". Today that would assert since we have cases where a JT waits on a nosafepoint lock without being _thread_blocked, mainly os::create_thread() and JfrThreadSampler::on_javathread_suspend() from some tests I run. VMThread::wait_for_vm_thread_exit() and ThreadsSMRSupport::wait_until_not_protected() also assert but in those cases the JT was already removed from the JT list so it wouldn't be blocking safepoints.
>
> Thank you for the explanation and further experiments @pchilano . 
> 
>>VMThread::wait_for_vm_thread_exit() and ThreadsSMRSupport::wait_until_not_protected() also assert but in those cases the JT was already removed from the JT list so it wouldn't be blocking safepoints.
> 
> I wonder if the check should be is_active_Java_thread instead?
> 
> The Service_lock, EscapeBarrier_lock, MonitorDeflation_lock, Notification_lock, and CodeSweeper_lock are held with a TBIVM (same mechanism).  These locks are fairly low level locks and don't check for safepoints.
> 
> I think the problematic scenario that Patricio is referring to is:
> Thread1: TBIVM (state == _thread_blocked), comes out of wait so owns Service_lock, does things while safepoint safe
> Thread2: waiting on Service_lock, so blocking safepoint
> But then Thread1 will come back to wait when done with things, Thread2 will proceed then get to it's safepoint check when done with eg. Service_lock.

I'm also trying the experiment that Patricio refers to above:

    +    } else if (thread->is_active_Java_thread() && rank() <= Mutex::nosafepoint) {
    +      // Don't allow JavaThread to wait on non-safepoint checking lock if not in a safe state.
    +      JavaThread* current = JavaThread::cast(thread);
    +      assert(current->thread_state() == _thread_blocked || current->thread_state() == _thread_in_native,
    +             "Cannot wait on active Java thread in unsafe state");

but the os::create_thread() case is an active Java thread.

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

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


More information about the hotspot-runtime-dev mailing list