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

[Coleen Phillimore]

On Fri, 5 Dec 2025 11:47:47 GMT, David Holmes <dholmes at openjdk.org> wrote:

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

I hadn't noticed the 'safe' variant of Thread::current_or_null, but it seems like it might be appropriate from the implementation if this can be called very early in VM initialization for ParkEvents.


inline Thread* Thread::current_or_null_safe() {
  if (ThreadLocalStorage::is_initialized()) {
    return ThreadLocalStorage::thread();
  }
  return nullptr;
}


It seems to cover more than the case of being called in a signal handler.  I think it's not a bad usage.  I was going to vote the other way until I saw the implementation.

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

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