RFR (XS) 6976636: JVM/TI test ex03t001 fails assertion

[Daniel D. Daugherty]

On 3/14/14 3:29 PM, serguei.spitsyn at oracle.com wrote:
> On 3/14/14 1:52 PM, Daniel D. Daugherty wrote:
>> On 3/13/14 3:24 PM, serguei.spitsyn at oracle.com wrote:
>>> Please, review the fix for:
>>>   https://bugs.openjdk.java.net/browse/JDK-6976636
>>>
>>>
>>> Open webrev:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2014/hotspot/6976636-JVMTI-unload.1 
>>>
>>
>> Thumbs up.
>
> Thank you a lot for the code review!
> The fix is simple but the issue is ugly and so, generates questions. :)
>
>>
>> src/share/vm/prims/jvmtiExport.cpp
>>     No comments.
>>
>> As for the extension call back stuff, I think the idea
>> is to pass a little more info about which thread motivated
>> the class unload. An agent might be interested in such a
>> statistic.
>
> Right.
> It is why I prefer to keep this callback parameter.
> BTW, the jdwp agent is posting synthetic class unload events.
> If I remember correctly, they have no jthread parameter.

There are several disagreements about what is in a JVM/TI
event versus what is in a JDI event. It can really mess up
your mind...

Dan


>
>>
>> > The class unload event is implemented as a source debug extension
>>
>> Not "source debug extension", but "JVM/TI extension event". The
>> source debug extension is a different thing (see JDI).
>
> Right.
> My head is broken, I keep using incorrect terms. :)
>
>>
>> > As a consequence of this fix the failing nsk.jvmti test ex03t001
>> > must be tweeked as well.
>>
>> Since you are changing an assert here, I'm curious why the test
>> needs to change. Does the test presume the jthread refers to
>> a real JavaThread?
>
> The test checks and fails if the NULL is returned.
> I'll file a bug with a necessary DKFL comment to recognize this
> failure mode until the SQE testbase build with the fix gets released.
>
>
> Thanks,
> Serguei
>
>>
>>
>> Dan
>>
>>
>>
>>>
>>> Summary:
>>>
>>>   This is a Nightly Stabilization issue.
>>>   The class unload event post in the JvmtiExport.cpp fails at the 
>>> assert
>>>   assuming the proxy thread causing the class unload must be a 
>>> JavaThread.
>>>   It is not the case when the proxy thread is a CMS ConcurrentGCThread.
>>>
>>>   This fix is to relax this check allowing this case.
>>>   The downside of this approach is that the jthread parameter passed 
>>> to the
>>>   even handler callback is NULL for the CMS thread.
>>>   This must be Ok as it would indicates that the proxy thread is a 
>>> CMS thread.
>>>
>>>   In fact, I have a doubt this parameter is needed at all.
>>>   So that an alternative approach could be to remove it from the 
>>> event callback.
>>>   My preference is to leave the jthread parameter as it is as it 
>>> does not impact anything.
>>>   The class unload event is implemented as a source debug extension 
>>> for the
>>>   sake of testing the extension functionality. The JDWP agent does 
>>> not use it.
>>>   The class unload events are not expected to be used by any 
>>> external tool.
>>>
>>>   As a consequence of this fix the failing nsk.jvmti test ex03t001 
>>> must be tweeked as well.
>>>   If review is positive then I'll file a bug and proceed with the 
>>> test fix.
>>>
>>> Testing:
>>>   Running the failing test: nsk/jvmti/scenarios/extension/EX03/ex03t001
>>>
>>>
>>> Thanks,
>>> Serguei
>>
>