RFR: 8366671: Refactor Thread::SpinAcquire and Thread::SpinRelease [v12]

[David Holmes]

On Fri, 5 Dec 2025 09:34:06 GMT, Anton Artemov <aartemov at openjdk.org> wrote:

>> src/hotspot/share/utilities/spinCriticalSection.hpp line 54:
>> 
>>> 52:   SpinCriticalSection(volatile int* lock)
>>> 53:     : _lock(lock)
>>> 54:       DEBUG_ONLY(COMMA _nsv(Thread::current_or_null_safe() != nullptr)) {
>> 
>> So we need to allow for null because this can be called early in VM init whilst the main thread is still attaching, but why do we need the "safe" variant? That is only for code that can be executed in a signal handling context
>
> Is this safety measure redundant though? I mean we move the code into a utility class, which may, potentially, end up being used in that context. It is not the case now, sure. If we do not allow that, we need document this and use a non-safe `current_or_null()`.

Well we moved it for convenience but we really don't want it to start being used all over. :) I don't like this kind of "just in case" code as it indicates to me that we don't really know how/where the code is being used - and where do you draw the line? This is debug code so the performance aspect is not a concern, but I prefer not to have it but if you like document that this is not for use in signal-handling context. But this isn't a deal-breaker if others think it worth being ultra-conservative here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28264#discussion_r2592416534