On 3/14/14 1:52 PM, Daniel D. Daugherty wrote:
On 3/13/14 3:24 PM, serguei.spit...@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
