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

[serguei.spitsyn at oracle.com]

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.

>
> > 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
>