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

[JC Beyler]

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.spitsyn at oracle.com <
serguei.spitsyn at 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.spitsyn at 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 <jcbeyler at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180824/44cd8d58/attachment.html>