RFR: 8295146: Clean up native code with newer C/C++ language features [v3]

[Julian Waters]

On Wed, 23 Nov 2022 21:36:24 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> I think the problem here is the friend declaration, which doesn't look like it's needed and could be deleted.
>
> Digging into this some more, the friend declaration exists to provide access to the private `os::win32::enum Ept`.
> 
> One obvious and cheap solution to that would be to make that enum public.  I think that would be an improvement vs the current friend declaration.  But there are some other things one could complain about there, such as the type of the function requiring a complicated function pointer cast where it's used.  Here's a patch that I think cleans this up.
> 
> 
> diff --git a/src/hotspot/os/windows/os_windows.cpp b/src/hotspot/os/windows/os_windows.cpp
> index 0651f0868f3..bf9e759b1d6 100644
> --- a/src/hotspot/os/windows/os_windows.cpp
> +++ b/src/hotspot/os/windows/os_windows.cpp
> @@ -511,7 +511,9 @@ JNIEXPORT
>  LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo);
>  
>  // Thread start routine for all newly created threads
> -static unsigned __stdcall thread_native_entry(Thread* thread) {
> +// Called with the associated Thread* as the argument.
> +unsigned __stdcall os::win32::thread_native_entry(void* t) {
> +  Thread* thread = static_cast<Thread*>(t);
>  
>    thread->record_stack_base_and_size();
>    thread->initialize_thread_current();
> @@ -744,7 +746,7 @@ bool os::create_thread(Thread* thread, ThreadType thr_type,
>      thread_handle =
>        (HANDLE)_beginthreadex(NULL,
>                               (unsigned)stack_size,
> -                             (unsigned (__stdcall *)(void*)) thread_native_entry,
> +                             &os::win32::thread_native_entry,
>                               thread,
>                               initflag,
>                               &thread_id);
> diff --git a/src/hotspot/os/windows/os_windows.hpp b/src/hotspot/os/windows/os_windows.hpp
> index 94d7c3c5e2d..197797078d7 100644
> --- a/src/hotspot/os/windows/os_windows.hpp
> +++ b/src/hotspot/os/windows/os_windows.hpp
> @@ -36,7 +36,6 @@ typedef void (*signal_handler_t)(int);
>  
>  class os::win32 {
>    friend class os;
> -  friend unsigned __stdcall thread_native_entry(Thread*);
>  
>   protected:
>    static int    _processor_type;
> @@ -70,6 +69,10 @@ class os::win32 {
>    static HINSTANCE load_Windows_dll(const char* name, char *ebuf, int ebuflen);
>  
>   private:
> +  // The handler passed to _beginthreadex().
> +  // Called with the associated Thread* as the argument.
> +  static unsigned __stdcall thread_native_entry(void*);
> +
>    enum Ept { EPT_THREAD, EPT_PROCESS, EPT_PROCESS_DIE };
>    // Wrapper around _endthreadex(), exit() and _exit()
>    static int exit_process_or_thread(Ept what, int exit_code);

The issue with that would be that thread_native_entry is declared as static to the compilation unit on other other Operating Systems as well, and having it as a static member on the win32 class instead would end up breaking this convention, for which I'm not sure if there's a reason why all of them are declared like this

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

PR: https://git.openjdk.org/jdk/pull/11081