Re: RFR (S) 8203356: VM Object Allocation Collector can infinite recurse

Fri, 24 Aug 2018 09:56:36 -0700

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

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/

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


Reply via email to