JNI WeakGlobalRefs

[David Holmes]

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
>