RFR: 8341413: Stop including osThread_os.hpp in the middle of the OSThread class

[Axel Boldt-Christmas]

On Wed, 2 Oct 2024 13:50:01 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

> Some HotSpot files have an interesting include pattern where the platform dependent code is included straight into the class containing the shared implementation.
> 
> This has various interesting effects to the code layout and readability of the code. I propose we stop doing this, where possible, and instead clearly separate the shared code and the platform specific code in separate classes. This then allows us to use the standard include patterns that we use for most of our code.
> 
> This PR is a suggestion for how to untangle this for the OSThread class.
> 
> Things in the code that changed with this patch that might be good to take an extra look at:
> 1) I dropped unnecessary includes
> 2) `pd_initialize/pd_destroy` was converted into constructor/destructor
> 3) Member initialization lists are used. Note that some variables that used to be uninitialized are now getting initialized. `_caller_sigmask` is one of the interesting once, because it's sizable array. I still don't think that is problematic because all the other code we run to start threads, but if I get push back on this I'll comment it out.
> 4) (3) switched the order of the `new Monitor` call and the `sigemptyset` call
> 5) I did some reordering of functions to unify the four platforms
> 6) Moved `_thread_id` to the platform files
> 7) I stopped exposing the `thread_id_t` typedef.
> 8) I created a virtual `thread_id_for_printing` function for those calls that want a unified integer type that can be printed. An alternative to this could be to use a non-virtual call, but that requires us to to define `OSThreadBase::thread_id_for_printing` in the platform files. 
> 
> Tested: tier1-3, (excluding AIX, which I can't build/test)

> > Even though I am not sure about the usefulness of `AllocFailStrategy::RETURN_NULL`, and whether if it is ever recoverable to fail (smallish) native allocations in Hotspot. Seems like we will crash very soon in any case.
> 
> The problem with allocations within the constructor is that we have no way to convey to the caller that we had a failure.

Setting a bool field during construction is something we do in a lot of places to signal if the construction was successful.

```c++
  OSThread* osthread = new (std::nothrow) OSThread();
  if (osthread == nullptr || osthread->has_constructor_failed()) {
    delete osthread;
    return false;
  }

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

PR Comment: https://git.openjdk.org/jdk/pull/21306#issuecomment-2390909425