RFR: 8313202: MutexLocker should disallow null Mutexes
Thomas Stuefe
stuefe at openjdk.org
Thu Jul 27 05:31:39 UTC 2023
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)
I think this change makes sense. Especially something like ReentrantLocker. Maybe we can make the syntax a bit more palatable?
A variant would be to give the existing MutexLocker a boolean template parameter, default to false, "allowNull". Paths that allow nullptr must opt in by specifying the parameter directly.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15043#issuecomment-1652937383
More information about the hotspot-dev
mailing list