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

[Alex Menkov]

Hi Serguei,

Maybe resolve code duplication in post_compiled_method_load(nmethod *nm) 
and post_compiled_method_load(JvmtiEnv* env, nmethod *nm)?

Looks like the only difference is logging, but I don't think it's 
important as old post_compiled_method_load(JvmtiEnv* env, const 
jmethodID method, const jint length, ...) was not used.

--alex

On 04/09/2019 13:58, 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 
>> <http://cr.openjdk.java.net/%7Esspitsyn/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 
>> <mailto:serguei.spitsyn at oracle.com> <serguei.spitsyn at oracle.com 
>> <mailto: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/
>>     <http://cr.openjdk.java.net/%7Esspitsyn/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
>