RFR(MS): 8222072: JVMTI GenerateEvents() sends CompiledMethodLoad events to wrong jvmtiEnv

[Jean Christophe Beyler]

Hi Serguei,

Looks good to me now :)
Jc

On Tue, Apr 9, 2019 at 1:58 PM <serguei.spitsyn at oracle.com> wrote:

> Hi Jc,
>
> Thank you a lot for looking at this!
>
> On 4/9/19 9:29 AM, Jean Christophe Beyler wrote:
>
> Hi Serguei,
>
> I saw a nit here:
>
>
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.1/test/hotspot/jtreg/serviceability/jvmti/GenerateEvents/MyPackage/GenerateEventsTest.java.html
>
> arei -> are
>
>
> Nice catch, fixed.
>
> - I'm not sure you needed two files for the agents, you could have re-used
> some of the code and just called twice GetEnv but that is a detail.
>
>
> Right.
> I initially wanted to use this way but then decided to use two real agent
> libraries.
> Wanted to exercise this path.
>
> - I thought JNIEnv was not supposed to be really kept because it should
> not be used by another thread, is there not a risk that you are doing that?
> It doesn't look like it but I've been surprised in the past
>
>
> You are right.
> Tried to fix it in new webrev version below.
>
>
> - Isn't there a chance that your second agent gets a normal JVMTI_EVENT_COMPILED_METHOD_LOAD
> before the GenerateEvents call and increments its counter?
>    - I guess we'd see if it becomes flaky at some point? :)
>
> Yes, it is expected.
> But they should be posted on threads other than Main thread.
> The callback should ignore them.
>
>
> The updated fix version is:
>
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.2/
>
>
> Thanks!
> Serguei
>
>
> Thanks!
> Jc
>
>
> On Mon, Apr 8, 2019 at 12:29 PM serguei.spitsyn at oracle.com <
> serguei.spitsyn at oracle.com> wrote:
>
>> Please, review a fix for:
>>   https://bugs.openjdk.java.net/browse/JDK-8222072
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.1/
>>
>>
>> Summary:
>>   The JVMTI GenerateEvents() must send CompiledMethodLoad events only to
>> the agent which called it.
>>   However, it sends events to all agents (jvmti environements) which
>> violates the JVMTI spec.
>>   The webrev above fixes this issue and adds new jtreg test:
>>     test/hotspot/jtreg/serviceability/jvmti/GenerateEvents
>>
>>
>> Testing:
>>   Mach5 submission for:
>>    - JVMTI tests: open/test/hotspot/jtreg/vmTestbase/nsk/jvmti
>>    - new test: test/hotspot/jtreg/serviceability/jvmti/GenerateEvents
>>
>> Thanks,
>> Serguei
>>
>>
>
> --
>
> Thanks,
> Jc
>
>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190409/0badf3d4/attachment.html>