RFR(10)(M): JDK-8164563: Test nsk/jvmti/CompiledMethodUnload/compmethunload001 keeps reporting: PRODUCT BUG: class was not unloaded in 5

[coleen.phillimore at oracle.com]

On 5/5/17 2:29 AM, serguei.spitsyn at oracle.com wrote:
> Hi Chris,
>
> The fix looks good.
> I like the simplification.
> Great bug hunt, Chris!

+1.  Great job figuring out this bug!
Coleen

>
> Thanks,
> Serguei
>
>
> On 5/4/17 15:38, Chris Plummer wrote:
>> Hello,
>>
>> Please review the following changes:
>>
>> http://cr.openjdk.java.net/~cjplummer/8164563/webrev.00/webrev.hotspot/
>> https://bugs.openjdk.java.net/browse/JDK-8164563
>>
>> There was an issue with CompileMethodUnload events not getting 
>> delivered. Short story is the root cause was the 
>> JvmtiDeferredEventQueue::_pending_list, which upon further review was 
>> deemed unnecessary, so all code related to it has been pulled.
>>
>> The _pending_list is a temporary list for CompileMethodUnload events 
>> that occur while at a safepoint. Putting them directly on the 
>> JvmtiDeferredEventQueue was thought not to be allowed at a safepoint, 
>> because doing so required acquiring the Service_lock, and it was 
>> thought that you can no do that while at a safepoint.
>>
>> The issue with the _pending_list is that it only gets processed if 
>> there is a subsequent JvmtiDeferredEventQueue::enqueue() call. For 
>> the test in question, this was not always happening. The test sits in 
>> a loop waiting for the unload events, but unless it triggers some 
>> compilation during this time, which results in enqueue() being called 
>> for the CompileMethodLoad event, it will never see the 
>> CompileMethodUnload events. It eventually gives up and fails. Most 
>> times however there is a compilation triggered while in the loop so 
>> the test passes.
>>
>> The first attempted solution was to use a VM op that triggered 
>> processing of the _pending_list. However, this also ran up against 
>> the issue of not being able to grab the Service_lock while at a 
>> safepoint because even _concurrent VM ops can end up being run 
>> synchronously and at a safepoint if you are already in a VMThread.
>>
>> After further review of the safepoint concern with the Service_lock, 
>> it was determined that it should be ok to grab it while at a 
>> safepoint, thus removing the need for the _pending_list. So basically 
>> the fix is to remove all _pending_list code, and have 
>> nmethod::post_compiled_method_unload() always directly call 
>> JvmtiDeferredEventQueue::enqueue().
>>
>> I tested by running the failing test at least 100 times on all 
>> supported platforms (it used to fail with a fairly high frequency). I 
>> also ran our other CompileMethodUnload and CompileMethodLoad tests 
>> about 100 times, and ran our full jvmti test suite a few times on 
>> each platform, along with the jck/vm/jvmti tests.
>>
>> thanks,
>>
>> Chris
>