RFR: 8274282: Clarify special wait assert [v2]

[Coleen Phillimore]

On Mon, 4 Oct 2021 19:33:47 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> 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.

I can't easily add this new assert. It's going to take a lot more code so should be done in a separate RFE.

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

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