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

[coleen.phillimore at oracle.com]

Thanks Erik!
Coleen

On 12/3/19 7:48 AM, erik.osterlund at oracle.com wrote:
> 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
>>
>