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

[Stefan Karlsson]

Thanks, Per!

StefanK

On 2017-10-18 14:18, Per Liden wrote:
> 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