Thanks for the review Serguei and nice catches! Here is the webrev with your comments fixed: http://cr.openjdk.java.net/~jcbeyler/8203356/webrev.01/
Let me know what you think, Jc On Fri, Aug 24, 2018 at 9:55 AM serguei.spit...@oracle.com < serguei.spit...@oracle.com> wrote: > 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. > > Webrev: http://cr.openjdk.java.net/~jcbeyler/8203356/webrev.00/ > Bug: https://bugs.openjdk.java.net/browse/JDK-8203356 > > Thanks! > Jc > > On Thu, Aug 2, 2018 at 12:46 PM JC Beyler <jcbey...@google.com> wrote: > >> 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/ >> Bug: https://bugs.openjdk.java.net/browse/JDK-8203356 >> >> 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. >> >> Thanks, >> Jc >> > > > -- > > Thanks, > Jc > > > > -- Thanks, Jc
