os_windows.cpp : simplify is_thread_cpu_time_supported ?

[Baesken, Matthias]

Hi David, thanks for the info .

I found  https://docs.microsoft.com/en-us/windows/win32/procthread/thread-security-and-access-rights
  so it looks like  we  need  THREAD_QUERY_INFORMATION or THREAD_QUERY_LIMITED_INFORMATION access right   for  GetThreadTimes .  

On the other hand ,   the test in os::is_thread_cpu_time_supported()   on Windows      might  (temporary ?)  fail for  other reasons too , it  is not clear to me if this  is really always 
 related to the wrong access rights ?

And at some places in HS code like  jfrThreadCPULoadEvent.cpp  ,    os::thread_cpu_time  is called anyway without checking  for  os::is_thread_cpu_time_supported()  ;
 same for thread.cpp  / Thread::print_on  but this is just printing some output so it is most likely not really a big issue  on Windows .

Best regards, Matthias




>> On 25/03/2021 11:49 pm, Baesken, Matthias wrote:
>>> Hello,  I wonder , should we just return  true  in  
>>> os::is_thread_cpu_time_supported()   on Windows  ?
>>>
>>> See
>>>
>>> https://github.com/openjdk/jdk/blob/master/src/hotspot/os/windows/os_windows.cpp#L4588 
>>>
>>> According to  MSDN
>>> https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getthreadtimes 
>>>
>>> GetThreadTimes is supported  on Win2003/XP and higher . This should be 
>>> fine for OpenJDK .
>> 
>> Yes it should be fine. There may be other Windows archaisms in the code 
>> that could be cleaned up now.
>
>The issue was not API availability (we got rid of that check a long time 
>ago) but security permissions. We actually just returned "true" prior to 
>JDK 5 but that was changed by JDK-4884677 when the JVM TI support was added.
>
>It is a bit messy. We use the result of 
>os::is_thread_cpu_time_supported() at initialization time, on the main 
>thread to then decide the global availability of this feature. And via 
>the normal launcher that thread will have all access bits set and so we 
>will flag thread_cpu_time as being available. At runtime we might 
>encounter a thread for which the access bits are not present and so the 
>actual get_thread_cpu_time call may return -1. In theory the JVM could 
>be loaded on a thread without full permissions and so we would then 
>globally disable thread_cpu_time.
>
>So I think this code has to stay.