[External] : Re: JNI WeakGlobalRefs

[Erik Osterlund]

Hi Hans,

On 23 Jul 2021, at 01:12, Hans Boehm <hboehm at google.com> wrote:



Thanks for the detailed response! More below ...

On Thu, Jul 22, 2021 at 1:20 AM Erik Osterlund <erik.osterlund at oracle.com<mailto:erik.osterlund at oracle.com>> wrote:
Hi Hans,

So you are saying that a jweak allows you to peek into the finalizer graph of a
finalizing object from the outside due to having phantom semantics, and that
is sometimes bad. I agree. As you noted, if you don't use finalizers, there is no
problem here.

But let's assume we do what you propose, and make jweak have weak semantics,
instead of phantom semantics. First of all, have we now removed all gimmicks
that allow you to peek through finalizers? I would say no.

Consider the following (same notation as your example):

F ---> ... ---> F

Here we have an object with a finalizer that can reach another object with a
finalizer. They can both get finalized by the same GC cycle. So depending on who
is enqueued for finalization first, you may or may not be able to peek into an
already finalized object here, from the outside, through another finalizer.
One might argue about the probability of users using an object behind a jweak
vs finalizer, without knowing their implementation. But it's bad news for both.
Once again, if finalizers are not used, there is no problem.

Agreed. I think this is problematic as well, and we're clearly not going to fix
that problem at this stage. But as I tried to argue too briefly, I think there kind
of is a work-around in the finalizer ordering case. If the first F's
finalizer keeps a strong reference (from some strongly reachable
data structure) to everything the finalizer needs to run, and clears it in
the finalizer. (IIRC, Guy Steele suggested this long ago, and I think the
2005 memory model work kind of, sort of, enabled this to be done correctly.)

In the WeakGlobalRef case, I don't think there is an analogous workaround.
So I think it's actually worse than what is already a very unpleasant problem.

I think you are right. But if we flipped the coin and bought the problem of native components being unable to access their Java mirror any longer, if they are finalizably reachable, which they can’t know, there wouldn’t really be any workaround for that either, AFAICT. So either way you end up with problems hard to work around, except by removing finalizers of course. Which by all means is what we want people to do.


Secondly, as for when you would ever want jweak to have phantom semantics...
I don't think this is uncommon, in particular for native code. It is not uncommon
to use JNI to implement a native mirror component for a Java object. The Java
object and its native data structure are to be considered as one logical unit that
stick together. The Java object has a pointer to its native part, and the native part
wants a pointer back to its Java object, so they can talk to each other seamlessly.
You don't want the back pointer to be strong because it would create memory leaks,
so you use a jweak. The expectation is that surely, as long as this Java object is
around, you can access it through the jweak. Especially since the spec says so, it's
a very sound assumption. Then some cleaner will delete the data structure when
it is no longer relevant. So in contexts where you are in the native data structure
and haven't passed around the object reference, you can always get the object
through the jweak as long as it isn't dead.
This sounds like a very reasonable intent, but I don't see how to implement it
correctly with the current semantics, unless you control the implementation of the
Java object all the way down to the bits, which seems rare.  The core question is
what it means for the Java object to "be around". When the Java object is eligible
for finalization, it's, in general, no longer safe to access from the native mirror, in
spite of the fact that the native mirror has no way to tell that it has entered that
half-dead-and-potentially-invalid state. The only way to dodge the issue is to
know that no part of the Java object is finalizable (or affected by
WeakReference-based cleanup), which I think is usually unknowable. It's
unclear to me what guarantees even the standard library provides in this respect.

I disagree. I don’t think you need to know the object isn’t finalizable. You just need to know your native mirror object doesn’t have a finalizer which breaks the object. And that seems easy to know. Just follow the guidelines and don’t write a finalizer for that object, and objects you use in your native code. Instead use a phantom based cleaner. If other objects have finalizers that can reach said native mirrored objects, doesn’t really matter. Because said finalizers are not allowed to break the object. So you only need to reason about your own class to know your native mirrored component works correctly.


Now if we change the semantics of jweak to weak instead, then every time the
Java object is reachable through a finalizer only, the logic will be wrong. The native
part thinks the object is dead, but it isn't. So if anyone uses this object through
a finalizer, bad things can happen. Kind of like a native object monitor not working
properly in finalizers. You essentially no longer can have an object with a native
component point at each other, without running into a bunch of issues. Either
introducing a memory leak, or having the object be crippled when reachable from
finalizers. For similar reasons, all native weak references used internally in HotSpot,
do have phantom semantics, and rely on that being the case.
That's a good data point, and suggests that current implementations need to keep
supporting the current semantics for internal use. But I think Java implementations
themselves here are in a completely different situation from client code:
I would hope that those HotSpot uses actually are aware of the referent implementation
all the way down to the bits, and either know there no finalizers involved, or write the
finalizer code to defend against accessing finalized objects. The client code, on the
other hand, seems extremely likely to use referents (e.g. listeners for some event
generated by native code)  that can rely on arbitrary unknown Java objects. And I
think the current semantics just don't work for such cases.

Similarly to my comment above, we just don’t need to know about what libraries use finalizers. We know our native components are implemented right with corresponding phantom based cleanups and don’t get broken by other finalizers.

But you may well be right that there is just enough client code using this correctly
that we can't actually change the semantics.

I think so. Especially if we only swap problems around. If we could make everyone happy, it might be a different story.


So it's a two edged sword I think. What is true for this whole discussion though is
that if we don't have finalizers, then there isn't really a problem. Finalizers are
deprecated, so hopefully its use will die out over time. What is also true is that
by changing the semantics of jweak, you trade one problem for another one.
This seems particularly nasty to change, as the spec clearly states what users
can expect from this API.
The failure probability indeed goes down dramatically if you get rid of finalizers.
Based on what I've seen, that's unfortunately not likely to happen quickly.

I can imagine the transition will be slow indeed.

But writing demonstrably correct code seems to remain a problem even without finalizers.
Based on the current spec, you can get a similar effect to a finalizer by
enqueuing a WeakReference. In reality, it might be the case that in the absence
of finalizers, GlobalWeakRefs are cleared when WeakReferences are enqueued,
But I don't think the spec says that. The implementation is still allowed to enqueue
WeakReferences, leave the object around for a while, and clear PhantomReferences
and GlobalWeakRefs later, leaving a window during which WeakReferences
have been enqueued and processed, but GlobalWeakRefs will continue to generate
strong references to the object.

Without finalizers, weak and phantom will be equivalent. Therefore jweak and and WeakReference will agree about whether the object is cleared or not. So after the WeakReference cleanup has run, jweaks are guaranteed to resolve to null. So I don’t see the problem you are referring to, in a finalizer absent world.

So I think this problem will really only go away after finalizers die out, and the spec
is updated to take advantage of the fact that finalizers are dead. Which I expect to
take a long time.

It sounds like we agree that without finalizers everyone will be happy. It’s just getting there that could take a while.

If we can't change the semantics, do you agree that it makes sense to have the spec warn
about the problem, and recommend a (strong) GlobalRef to a WeakReference for
references to objects not controlled by the same developer?

Writing down a note that you should probably think about this, and possibly recommending removal of finalizers, seems fine to me.

/Erik

Hans


Hope this helps.

/Erik

> [ Moving here from core-libs-dev on David Holmes' recommendation. ]
>
> 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