On Wed, 23 Nov 2022 21:36:24 GMT, Kim Barrett <kbarr...@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