RFR: 8313202: MutexLocker should disallow null Mutexes

[David Holmes]

On Wed, 26 Jul 2023 17:06:02 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> As seen in [JDK-8313081](https://bugs.openjdk.org/browse/JDK-8313081), it is fairly easy to pass nullptr `Mutex` to `MutexLocker` by accident, which would just silently avoid the lock.
> 
> There are a few places in Hotspot where we pass `nullptr` to simulate re-entrancy and/or conditionally take the lock. Those places can be more explicit, and the default `MutexLocker` can disallow nullptrs for extra safety.
> 
> Open for some bikeshedding on the names of the new `MutexLockers`. Particularly `ReentrantMutexLocker` might lull readers into believing it does safepoint checks on re-entrant "lock", which it actually does not do.
> 
> More thorough testing with different GC/JIT combinations is running now, we might find more issues there. Meanwhile, please comment on the approach.
> 
> Additional testing:
>  - [x] `grep -R "MutexLocker " src/hotspot | grep -i null`, no hits
>  - [x] `grep -R "MutexLocker " src/hotspot | grep -i ?`, no hits
>  - [x] Linux AArch64 fastdebug, `tier1 tier2 tier3` (re-run in progress)

Mutexes can be used in generic code that might be used before VM init reaches the point of being multi-threaded, as well as being used after that. The former case is transparently handled because the mutex ptr at the point is null and so no locking is attempted, nor is it needed. If we are going to go back in time then just reinstate MutexLockerEx - lets not go down this current path of over complexifying the mutex code!

In many (all?) cases where you may need a check for a non-null mutex you more generally want a subsystem initialization check - which can be done once. Adding null-checks to nearly every mutex use is not at all beneficial IMHO.

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

PR Comment: https://git.openjdk.org/jdk/pull/15043#issuecomment-1653054230