RFR: 8212184: Incorrect oop ref strength used for referents in FinalReference
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
Hi Per, Looks good. Thanks, /Erik On 2018-10-15 16:42, Per Liden 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
On Oct 15, 2018, at 10:42 AM, Per Liden <per.liden@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?
Hi Kim, On 10/15/2018 08:55 PM, Kim Barrett wrote:
On Oct 15, 2018, at 10:42 AM, Per Liden <per.liden@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
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@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
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@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
On Oct 31, 2018, at 11:35 AM, Per Liden <per.liden@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:
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@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
Thanks for reviewing, Erik and Kim! /Per On 10/31/2018 05:50 PM, Kim Barrett wrote:
On Oct 31, 2018, at 11:35 AM, Per Liden <per.liden@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:
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@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
participants (3)
-
Erik Österlund
-
Kim Barrett
-
Per Liden