On Thu, 19 Nov 2020 10:10:06 GMT, Serguei Spitsyn <sspit...@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
