RFR: 8212184: Incorrect oop ref strength used for referents in FinalReference

[Kim Barrett]

> On Oct 31, 2018, at 11:35 AM, Per Liden <per.liden at oracle.com> 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

Looks good.

> 
> 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