|
Look good, thank you for the update!
Thanks,
Serguei
On 8/24/18 14:58, JC Beyler wrote:
Thanks for the review Serguei and nice catches!
Here is the webrev with your comments fixed:
Let me know what you think,
Jc
One more
suggestion for simplification:
56 char *generic = NULL;
57
58 (*jvmti)->GetClassSignature(jvmti, klass, &signature, &generic);
59
The value returned in generic is not used.
You can pass NULL in place of @generic and get rid of
line #56.
Thanks,
Serguei
On 8/24/18 09:41, serguei.spit...@oracle.com
wrote:
Hi Jc,
It looks good in general, thank you for taking care
about this!
A couple of comments though.
http://cr.openjdk.java.net/%7Ejcbeyler/8203356/webrev.00/test/hotspot/jtreg/serviceability/jvmti/VMEvent/libVMEventTest.c.html
58 (*jvmti)->GetClassSignature(jvmti, klass, &signature, &generic);
59
60 if (JNI_ENV_PTR(jni)->ExceptionOccurred(JNI_ENV_ARG(jni))) {
61 JNI_ENV_PTR(jni)->FatalError(
62 JNI_ENV_ARGS2(jni, "Failed during the GetClassSignature call"));
63 }
The JVMTI error code, returned by GetClassSignature has to be checked, not JNI ExceptionOccurred.
Also, I'd suggest to check for signature to be non-NULL.
Also, the indent in the VMEventRecursionTest.java has to be 4 (as we normally use for java code), not 2.
Thanks,
Serguei
On 8/22/18 16:20, JC Beyler wrote:
Hi all,
Would anyone want to look at this change? It
helps fix a minor bug if someone provokes a VM
allocation during a VM Allocation Event.
Thanks!
Jc
Hi all,
(Renaming the thread that did not have the
RFR in front of the subject, I apologize)
Could someone review this change:
Webrev: http://cr.openjdk.java.net/~jcbeyler/8203356/webrev.00/
Basically, if during a callback from
a VMObjectAlloc event, the user provokes a
clone, the code would send a new callback and
you can recurse infinitely.
I added a test that fails without the fix
and passes now.
--
--
|
