RFR (11): 8205878: pthread_getcpuclockid is expected to return 0 code

[Chris Plummer]

Hi David,

Solaris problems aside, overall it looks fine. Some minor things I noted:

I noticed that exitCode is never modified in agentA() or agentB(), so 
there isn't much point to having it. If you reach the bottom of the 
function, it passed, so PASSED can be returned. The code would be more 
clear if it did this. As-is it is implied that you can reach the bottom 
when it fails.

Is detaching the threads along the failure paths really needed? exit() 
is called, so this would seem to make it unnecessary.

I prefer assignments not to be embedded inside the "if" condition. The 
DetachCurrentThread code in THREAD_return() is much more readable than 
the similar code in agentA() and agentB().

In the test:

   54         // Generally as long as we don't crash of throw unexpected
   55         // exceptions then the test passes. In some cases we know 
exactly

"of" should be "or".

Shouldn't you be catching exceptions for all the Thread methods you are 
calling? Otherwise the test will exit if one is thrown, and the above 
comment indicates that you don't want this.

Don't we normally put these tests in a package?

thanks,

Chris

On 7/5/18 2:58 AM, David Holmes wrote:
> <sigh> Solaris compiler complains about doing a return from inside a 
> do-while loop. I'll have to rework part of the fix tomorrow.
>
> David
>
> On 5/07/2018 6:19 PM, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8205878
>> Webrev: http://cr.openjdk.java.net/~dholmes/8205878/webrev/
>>
>> Problem:
>>
>> The tests create native threads that attach to the VM through 
>> JNI_AttachCurrentThread but which then terminate without detaching 
>> themselves. When the VM exits and we're using Flight Recorder 
>> "dumponexit" this leads to a call to VM_PrintThreads that in part 
>> wants to print the per-thread CPU usage. When we encounter the 
>> threads that have terminated already the low level 
>> pthread_getcpuclockid calls returns ESRCH but the code doesn't expect 
>> that and so fails an assert in debug mode and can SEGV in product mode.
>>
>> Solution:
>>
>> Serviceability-side: fix the tests
>>
>> Change the tests so that the threads detach before terminating. The 
>> two tests are (surprisingly) written in completely different styles, 
>> so the solution also takes on two different styles.
>>
>> Runtime-side: make the VM more robust in the fact of JNI attached 
>> threads that terminate before detaching, and add a regression test
>>
>> I took a good look at the low-level code for interacting with 
>> arbitrary threads and as far as I can see the problem only exists for 
>> this one case of pthread_getcpuclockid on Linux. Elsewhere the 
>> potential for a library call failure just reports an error value 
>> (such as -1 for the cpu time used).
>>
>> So the fix is simply to allow for ESRCH when calling 
>> pthread_getcpuclockid and return -1 for the cpu usage in that case.
>>
>> I created a new regression test to create a new native thread, attach 
>> it and then let it terminate while still attached. The java code then 
>> calls various Thread and ThreadMXBean functions on it to ensure there 
>> are no crashes or unexpected exceptions.
>>
>> Testing:
>>   - old tests with fixed run-time
>>   - old run-time with fixed tests
>>   - mach tier4 (which exposed the problem - that's where we enable 
>> Flight recorder for the tests) [in progress]
>>   - mach5 tier 1-3 for good measure [in progress]
>>   - new regression test
>>
>> Thanks,
>> David