RFR (M) 8212160: JVMTI agent crashes with "assert(_value != 0LL) failed: resolving NULL _value"

[erik.osterlund at oracle.com]

Hi Coleen,

This looks great. Thanks for sorting this out!

/Erik

On 12/2/19 3:43 PM, coleen.phillimore at oracle.com wrote:
>
>
> On 11/26/19 7:03 PM, David Holmes wrote:
>> (adding runtime as well)
>>
>> Hi Coleen,
>>
>> On 27/11/2019 12:22 am, coleen.phillimore at oracle.com wrote:
>>> Summary: Add local deferred event list to thread to post events 
>>> outside CodeCache_lock.
>>>
>>> This patch builds on the patch for JDK-8173361.  With this patch, I 
>>> made the JvmtiDeferredEventQueue an instance class (not AllStatic) 
>>> and have one per thread.  The CodeBlob event that used to drop the 
>>> CodeCache_lock and raced with the sweeper thread, adds the events it 
>>> wants to post to its thread local list, and processes it outside the 
>>> lock.  The list is walked in GC and by the sweeper to keep the 
>>> nmethods from being unloaded and zombied, respectively.
>>
>> Sorry I don't understand why we would want/need a deferred event 
>> queue for every JavaThread? Isn't this only relevant for 
>> non-JavaThreads that need to have the ServiceThread process the 
>> deferred event?
>
> I thought I'd written this in the bug but I had only discussed this 
> with Erik.  I've added a comment to the bug to explain why I added the 
> per-JavaThread queue.  In order to process these events after the 
> CodeCache_lock is dropped, I have to queue them somewhere safe. The 
> ServiceThread queue is safe, *but* the ServiceThread can't keep up 
> with the events, especially from this test case.  So the test case 
> gets a native OOM.
>
> So I've added the safe queue as a field to each JavaThread because 
> multiple JavaThreads could be posting these events at the same time, 
> and there didn't seem to be a better safe place to cache them, without 
> adding another layer of queuing code.
>
> I did write comments to this effect here:
>
> http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev/src/hotspot/share/prims/jvmtiCodeBlobEvents.cpp.udiff.html 
>
>
> Thanks,
> Coleen
>
>>
>> David
>>
>>> Also, the jmethod_id field in nmethod was only used as a boolean so 
>>> don't create a jmethod_id until needed for post_compiled_method_unload.
>>>
>>> Ran hs tier1-8 on linux-x64-debug and the stress test that crashed 
>>> in the original bug report.
>>>
>>> open webrev at 
>>> http://cr.openjdk.java.net/~coleenp/2019/8212160.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8212160
>>>
>>> Thanks,
>>> Coleen
>