RFR: 8274301: Locking with _no_safepoint_check_flag does not check NJT

[Coleen Phillimore]

On Thu, 14 Oct 2021 21:04:29 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> There are Mutexes that have a safepoint checking rank, that are locked via:
> MutexLocker ml(safepoint_lock, Mutex::_no_safepoint_check_flag);
> 
> These don't assert for the inconsistency because the thread that calls this is a NonJavaThread.
> 
> But if you lock
> MutexLocker ml(nosafepoint_lock);  // default safepoint check
> 
> These *will* assert for the safepoint checking inconsistency even for NonJavaThreads.
> 
> NonJavaThreads don't participate in the safepoint checking protocol.  And some of these safepoint checking locks are shared between java and nonjava threads. So in the existing code, there's a mix of locking with _no_safepoint_check_flag and not.
> 
> This has a wrinkle because if you have a safepoint checking lock, and call wait on it, we now have to handle the case that the caller is a NonJavaThread and should not safepoint check (just like the Mutex::lock call).
> 
> Maybe this shouldn't be fixed, and we should just document the discrepancy.
> 
> Tested with tier1-3.

Yes, it's true that the _no_safepoint_check is redundant but we check this for all non-safepointing lock acquisitions, just not safepointing ones.  Because we want to keep the check, I think it should check both classes of locks.

If we do enforce this MutexLocker parameter is consistent for safepointing checking locks, the MonitorLocker wait code does this:

  bool wait(int64_t timeout = 0) {
    return _flag == Mutex::_safepoint_check_flag ?
      as_monitor()->wait(timeout) : as_monitor()->wait_without_safepoint_check(timeout);
  }

So if we use MonitorLocker(safepoint_check_lock) and then wait on a safepoint checking lock, then the wait with safepoint check will be called on a NJT.  Which is why I added it.

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

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