RFR: 8212879: Make JVMTI TagMap table concurrent [v11]

[Coleen Phillimore]

On Thu, 19 Nov 2020 10:10:06 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/AttachOnDemand/attach021/attach021Agent00.cpp:
>>> 
>>> The change below is not needed as the call to nsk_jvmti_aod_disableEventAndFinish() does exactly the same:
>>> 
>>> ```
>>> -    nsk_jvmti_aod_disableEventAndFinish(agentName, JVMTI_EVENT_OBJECT_FREE, success, jvmti, jni);
>>> +
>>> +    /* Flush any pending ObjectFree events, which may set success to 1 */
>>> +    if (jvmti->SetEventNotificationMode(JVMTI_DISABLE,
>>> +                                        JVMTI_EVENT_OBJECT_FREE,
>>> +                                        NULL) != JVMTI_ERROR_NONE) {
>>> +        success = 0;
>>> +    }
>>> +
>>> +    nsk_aod_agentFinished(jni, agentName, success);
>>>  }
>>> ```
>> 
>> This change really is needed.
>> 
>> The success variable in the test is a global, initially 0, set to 1 by the
>> ObjectFree handler.
>> 
>> In the old code, if the ObjectFree event hasn't been posted yet, we pass the
>> initial 0 value of success to nsk_jvmti_aod_disabledEventAndFinish, where
>> it's a local variable (so unaffected by any changes to the variable in the
>> test), so stays 0 through to the call to nsk_aod_agentFinished.  So the test
>> fails. 
>> 
>> The split in the change causes the updated post-ObjectFree event success
>> value of 1 to be passed to agentFinished.  So the test passes.
>> 
>> That required some head scratching to find at the time.  That's the point of
>> the comment about flushing pending events.  Maybe the comment should be
>> improved.
>
> @kimbarrett
> Okay, thank you for explanation.
> I agree, it'd make sense to improve the comment a little bit.
> Thanks,
> Serguei

So should nsk_jvmti_aod_disableEventAndFinish pass the address of success instead?  Why didn't it fail before this change?

-------------

PR: https://git.openjdk.java.net/jdk/pull/967