RFR: 8215451: JNI IsSameObject should not keep objects alive

[David Holmes]

On 17/12/2018 2:59 pm, Kim Barrett wrote:
>> On Dec 15, 2018, at 5:41 AM, Per Liden <per.liden at oracle.com> wrote:
>>
>> JNI IsSameObject should resolve the JNIHandles its comparing without keeping the objects they point to alive. Otherwise, checking if a jweak is reachable by doing IsSameObject(jweak, NULL) will be counter productive, as it will make sure that jweak stays alive.
>>
>> This is currently only a problem when using ZGC, which does concurrent jweak processing.
>>
>> This was found by the "test/hotspot/jtreg/serviceability/jvmti/HeapMonitor/MyPackage/HeapMonitorThreadTest.java" test, which keeps a list of jweaks, which it will periodically iterate over and do IsSameObject(jweak, NULL) to determine if any of them have been cleared. The end result is the these jweaks will never be cleared and the heap eventually gets full and an OOME is thrown.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8215451
>> Webrev: http://cr.openjdk.java.net/~pliden/8215451/webrev.0
>>
>> /Per
> 
> This is applying AS_NO_KEEPALIVE for both strong and weak jobjects.
> I'm not sure that's intentional.  The CR and RFR only talk about
> jweaks.
> 
> I think there might be a race that makes this not work for ZGC.
> 
> I think (but perhaps I'm misremembering?) that only Shenandoah does
> anything interesting for Access::equals, with all other collectors in
> the JDK (including ZGC) just resolving down to an == on the arguments.
> If that's incorrect then the rest of this comment may be wrong.
> 
> Consider we have two jobject arguments, A and B, neither resolved (so
> neither contains a "good" value), and both referring to the same
> object that is not yet marked.
> 
> (1) resolve_no_keepalive(A) => the unmarked (unforwarded) value.
> (2) Some other thread resolves B, marking it and making B's value good.
> (3) resolve_no_keepalive(B) => the marked (forwarded) value.
> (4) result of (1) == result of (2) => false.
> 
> Obviously this isn't a problem for IsSameObject(jweak, NULL). I think
> what we really have is JNI weak having the same issue as
> j.l.r.Reference and likely needs a similar solution, e.g. JDK-8188055.
> The proposed Reference.isReferenceTo doesn't have the above race.
> 
> Responding to David - the resolve keeps the jweak's referent alive for
> the current collection cycle. It could still die in some later
> collection cycle, unless the described iteration happens again.
> JDK-8188055 is the same problem for j.l.r.Reference.

Okay - thanks. That seems undesirable. Unclear why this hasn't been an 
issue in the past.

David

>