RFR: 8212184: Incorrect oop ref strength used for referents in FinalReference
Erik Österlund
erik.osterlund at oracle.com
Wed Oct 31 15:39:03 UTC 2018
Hi Per,
Looks good.
Thanks,
/Erik
On 2018-10-31 16:35, Per Liden wrote:
> Hi,
>
> After some off-line discussions with Kim and Erik about whether this
> should be STRONG or PHANTOM it was agreed that it technically doesn't
> matter. So it comes down to preference. Kim (who preferred PHANTOM)
> agreed to go with STRONG (which Erik and I preferred) as long there
> was a comment explaining why. So I updated the patch accordingly:
>
> http://cr.openjdk.java.net/~pliden/8212184/webrev.1
>
> cheers,
> Per
>
>
> On 10/16/2018 01:55 PM, Per Liden wrote:
>> Hi Kim,
>>
>> On 10/15/2018 08:55 PM, Kim Barrett wrote:
>>>> On Oct 15, 2018, at 10:42 AM, Per Liden <per.liden at oracle.com> wrote:
>>>>
>>>> AccessBarrierSupport::resolve_unknown_oop_ref_strength() returns an
>>>> incorrect oop ref strength for referents in FinalReference objects.
>>>> It currently returns ON_WEAK_OOP_REF when it should return
>>>> ON_STRONG_OOP_REF. This is not really an issue for any GC except
>>>> ZGC when using the ZHeapIterator to walk the heap while
>>>> resurrection is blocked.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8212184
>>>> Webrev: http://cr.openjdk.java.net/~pliden/8212184/webrev.0
>>>>
>>>> /Per
>>>
>>> src/hotspot/share/gc/shared/accessBarrierSupport.cpp
>>> 32 if (!java_lang_ref_Reference::is_referent_field(base,
>>> offset) ||
>>> 33 java_lang_ref_Reference::is_final(base)) {
>>> 34 ds |= ON_STRONG_OOP_REF;
>>>
>>> This doesn't seem right. Doesn't this give the wrong answer for G1?
>>>
>>> I'm not even sure "strong" is the right answer for ZGC in the
>>> described context.
>>>
>>> What am I missing?
>>>
>>
>> There's no way for a mutator to get hold of the referent (except via
>> Unsafe, which we've said we don't care about anyway). The only time
>> we would randomly step on a referent in a Finalizer is when we're
>> doing heap iteration in ZGC.
>>
>> An alternative, to perhaps make this more explicit, would be to have
>> an ON_FINAL_OOP_REF, but it would end up doing the same thing as
>> ON_STRONG_OOP_REF.
>>
>> /Per
More information about the hotspot-gc-dev
mailing list