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

[Stefan Karlsson]

On Thu, 3 Oct 2024 01:39:21 GMT, David Holmes <dholmes at openjdk.org> wrote:

> I personally don't have an issue with the current technique to generate a single platform-specific `OSThread` class, but this refactoring is also okay.

Thanks for not blocking this suggestion.

> There is some unfortunate duplication of the boiler-plate code for the class but not too bad.

I pushed a change that moved the NONCOPYABLE macro. I did a little experiment with moving even more code to the OSThreadBase, but I'm not sure if this is wanted or not. I've put that change here in-case someone wants to take a look:
https://github.com/openjdk/jdk/compare/pr/21306...stefank:jdk:osThread_follow_up

> 
> I'm tempted to suggest pushing the `_startThread_lock` support into `OSThreadBase` under `#ifndef windows`, just to reduce some duplication. (I may also look at using that for Windows too in the near future, which would address it then.)

I'll leave this for next time someone wants to do a little bit of clean-up in this area.

One interesting thing that we found while looking at this is that the AIX port has the `_startThread_lock`, but it doesn't use it to coordinate the starting of the created thread. It's unclear to me if that's a bug in that port.

> 
> I could not see where `thread_type()` is actually used so possibly an additional cleanup opportunity there (not necessarily for this PR).

Maybe one reason to keep it is as an aid when debugging with a debugger? OTOH, while playing around with the patch linked above I found that the field is not available on Windows, and it's also left uninitialized when we call `create_attached_thread`.

> 
> I don't have any concerns with any of the items that you flagged.
> 
> Thanks

Thanks for reviewing!

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

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