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

[serguei.spitsyn at oracle.com]

Hi Chris,

The fix looks good.
I like the simplification.
Great bug hunt, Chris!

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