JNI WeakGlobalRefs

[Brian Goetz]

You might also consider bringing this to panama-dev, since that will 
eventually be the recommended replacement for most uses of JNI.

On 7/21/2021 6:25 PM, David Holmes wrote:
> Hi Hans,
>
> On 22/07/2021 7:54 am, Hans Boehm wrote:
>> Is this an appropriate list to discuss JNI?
>
> No - hotspot-dev (to get runtime and GC folk) is the place to discuss 
> JNI.
>
> Thanks,
> David
>
>> I'm concerned that the current semantics of JNI WeakGlobalRefs are still
>> dangerous in a very subtle way that is hidden in the spec. The current
>> (14+) spec says:
>>
>> “Weak global references are related to Java phantom references
>> (java.lang.ref.PhantomReference). A weak global reference to a specific
>> object is treated as a phantom reference referring to that object when
>> determining whether the object is phantom reachable (see java.lang.ref).
>> ---> Such a weak global reference will become functionally equivalent to
>> NULL at the same time as a PhantomReference referring to that same 
>> object
>> would be cleared by the garbage collector. <---”
>>
>> (This was the result of JDK-8220617, and is IMO a large improvement over
>> the prior version, but ...)
>>
>> Consider what happens if I have a WeakGlobalRef W that refers to a Java
>> object A which, possibly indirectly, relies on an object F, where F is
>> finalizable, i.e.
>>
>> W - - -> A -----> ... -----> F
>>
>> Assume that F becomes invalid once it is finalized, e.g. because the
>> finalizer deallocates a native object that F relies on. This seems to 
>> be a
>> very common case. We are then exposed to the following scenario:
>>
>> 0) At some point, there are no longer any other references to A or F.
>> 1) F is enqueued for finalization.
>> 2) W is dereferenced by Thread 1, yielding a strong reference to A and
>> transitively to F.
>> 3) F is finalized.
>> 4) Thread 1 uses A and F, accessing F, which is no longer valid.
>> 5) Crash, or possibly memory corruption followed by a later crash 
>> elsewhere.
>>
>> (3) and (4) actually race, so there is some synchronization effort 
>> and cost
>> required to prevent F from corrupting memory. Commonly the 
>> implementer of W
>> will have no idea that F even exists.
>>
>> I believe that typically there is no way to prevent this scenario, 
>> unless
>> the developer adding W actually knows how every class that A could 
>> possibly
>> rely on, including those in the Java standard library, are implemented.
>>
>> This is reminiscent of finalizer ordering issues. But it seems to be 
>> worse,
>> in that there isn't even a semi-plausible workaround.
>>
>> I believe all of this is exactly the reason PhantomReference.get() 
>> always
>> returns null, while WeakReference provides significantly different
>> semantics, and WeakReferences are enqueued when an object is enqueued 
>> for
>> finalization.
>>
>> The situation improves, but the problem doesn't fully disappear, in a
>> hypothetical world without finalizers. It's still possible to use
>> WeakGlobalRef to get a strong reference to A after a WeakReference to 
>> A has
>> been cleared and enqueued. I think the problem does go away if all 
>> cleanup
>> code were to use PhantomReference-based Cleaners.
>>
>> AFAICT, backward-compatibility aside, the obvious solution here is to 
>> have
>> WeakGlobalRefs behave like WeakReferences. My impression is that this 
>> would
>> fix significantly more broken clients than it would break correct 
>> ones, so
>> it is arguably still a viable option.
>>
>> There is a case in which the current semantics are actually the desired
>> ones, namely when implementing, say, a String intern table. In this case
>> it's important the reference not be cleared even if the referent is, at
>> some point, only reachable via a finalizer. But this use case again 
>> relies
>> on the programmer knowing that no part of the referent is invalidated 
>> by a
>> finalizer. That's a reasonable assumption for the
>> Java-implementation-provided String intern table. But I'm not sure it's
>> reasonable for any user-written code.
>>
>> There seem to be two ways forward here:
>>
>> 1) Make WeakGlobalRefs behave like WeakReferences instead of
>> PhantomReferences, or
>> 2) Add strong warnings to the spec that basically suggest using a strong
>> GlobalRef to a WeakReference instead.
>>
>> Has there been prior discussion of this? Are there reasonable use 
>> cases for
>> the current semantics? Is there something else that I'm overlooking? If
>> not, what's the best way forward here?
>>
>> (I found some discussion from JDK-8220617, including a message I posted.
>> Unfortunately, it seems to me that all of us overlooked this issue?)
>>
>> Hans
>>