8189360: JvmtiExport::weak_oops_do is called for all JNIHandleBlock instances

[Per Liden]

Looks good!

/Per

On 2017-10-18 14:01, Stefan Karlsson wrote:
> Hi Per,
>
> On 2017-10-17 23:43, Per Liden wrote:
>> Hi,
>>
>> On 2017-10-16 17:40, Stefan Karlsson wrote:
>>> Hi all,
>>>
>>> Please review this patch to move the call of the static
>>> JvmtiExport::weak_oops_do out of the JNIHandleBlock::weak_oops_do
>>> member function into the new WeakProcessor.
>>>
>>> Today, this isn't causing any bugs because there's only one instance
>>> of JNIHandleBlock, the _weak_global_handles. However, in prototypes
>>> with more than one JNIHandleBlock, this results in multiple calls to
>>> JvmtiExport::weak_oops_do.
>>>
>>> http://cr.openjdk.java.net/~stefank/8189360/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8189360
>>
>>    30 void WeakProcessor::unlink_or_oops_do(BoolObjectClosure*
>> is_alive, OopClosure* keep_alive, VoidClosure* complete) {
>>    31   JNIHandles::weak_oops_do(is_alive, keep_alive);
>>    32   if (complete != NULL) {
>>    33     complete->do_void();
>>    34   }
>>    35
>>    36   JvmtiExport::weak_oops_do(is_alive, keep_alive);
>>    37   if (complete != NULL) {
>>    38     complete->do_void();
>>    39   }
>>    40 }
>>
>> Should you really be calling complete->do_void() twice here. It seems
>> to me that doing it once, after both calls to weak_oops_do() would
>> mimic what the old code did?
>
> You're right. I've update the code to only call the "complete" closure
> at the end of the function.
>
> FYI, the latest revision of the patch for 8189360 also updated the name
> unlink_or_oops_do to weak_oops_do.
>
> I've also taken the liberty to implement oops_do as a call to
> weak_oops_do. This way we only have to list the calls to the individual
> containers once.
>
> New webrevs:
>  http://cr.openjdk.java.net/~stefank/8189360/webrev.01.delta
>  http://cr.openjdk.java.net/~stefank/8189360/webrev.01
>
> Thanks,
> StefanK
>
>>
>> cheers,
>> Per
>>
>>>
>>> This patch builds upon the patch in:
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-October/028684.html
>>>
>>>
>>> Tested with JPRT.
>>>
>>> Thanks,
>>> StefanK