RFR: 8257180: Prevent resurrection of final/phantom refs through JNI weak refs

[Kim Barrett]

[hotspot-runtime-dev was the wrong mailing list for this PR.]

> On Nov 26, 2020, at 12:52 PM, Roman Kennke <rkennke at openjdk.java.net> wrote:
> 
> It is currently possible to resurrect finalizable objects or even phantom references after they have been finalized:
> - Create JNI weak reference to object
> - Create phantom reference to object/ use object with finalize() method, so that a FinalReference is created.

It is true that JNI weak references can be used to resurrect
finalizable objects. It has "always" been so. (I think the JNI spec
was silent on the matter so implicitly vague prior to the 6/2001
Clarification; I couldn't find a copy of the spec from prior to that
to be sure though.)

8071507 (JDK 9) made it impossible for JNI weak references to access
the referent of a PhantomReference after it has been determined to be
phantom reachable.
https://bugs.openjdk.java.net/browse/JDK-8071507 

> As soon as the object is no longer strongly reachable, the Finalizer will invoke finalize() on the object, or, if it has been wrapped in a PhantomReference, that PR will be enqueued in its ReferenceQueue. With some luck, it is still possible to get hold of the original object via the JNI weak reference.
> 
> This is actually specified in JNI specification:
> https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#weak-global-references
> 
> […]
> 
> The implementation in referenceProcessor.cpp even has a comment about it:
> 
> // Weak global JNI references. […]

[That stale comment was removed as part of 8189359 (JDK 10).  It
should have been removed by 8071507, but was missed.]
https://bugs.openjdk.java.net/browse/JDK-8189359 

> (Notice: I don't have access to bug#4126360, I cannot tell if there is any interesting discussion about the usefulness there.)

I can't go into too much detail, and definitely can't open that bug.
But I can give some information.

There was discussion of changing the strength of JNI weak to be
equivalent to WeakReference, so stronger than finalization. This was
found to break some applications and frameworks.

There was also discussion of specifying the relative strengths of JNI
weak references and PhantomReference, making PhantomReference
explicitly the weakest. That could have been done, but it was decided
to instead make the order explicitly vague in the JNI specification;
hence the 6/2001 addition.

It was also noted that with native code you can do anything, so being
able to mess with finalization or PhantomReferences isn't a big deal;
untrusted native code can wreak all kinds of havoc.

> However, it turns out that the above clarification to JNI weak reference specification has been removed since Java 13:
> 
> https://docs.oracle.com/en/java/javase/13/docs/specs/jni/functions.html#weak-global-references
> 
> Which means that we are, strictly speaking, now free to rectify the situation. And since this 'feature' doesn't seem to have any useful applications, and only serves to shoot oneself into its own foot, I propose that we go ahead and do that.

The spec update in JDK 13
https://bugs.openjdk.java.net/browse/JDK-8188066 
only removed the explicit vagueness about the relationship between JNI
weak references and PhantomReferences. Instead, JNI weak references
are defined to have the same strength as PhantomReference. The
relationship between them and other reference types and finalization
remained unchanged.

That spec update should have happened in conjunction with 8071507, but
we (I) forgot, and didn't get around to it for a while. Mea culpa.

> This PR addresses the situation for ZGC and Shenandoah. I am not sure if we should try and do the other GCs too (in referenceProcessor.cpp).

Even if we wanted to make such a change to the behavior of JNI weak
references, the proposed code change is insufficient.  The backing
implementation of JNI weak references is weak OopStorage, and all weak
OopStorages are treated by all collectors as having phantom strength.
Apparently none of the tests you ran detect that problem.

We really do want the other weak OopStorage-based references to be
phantom strength.  Making JNI weak references different from the
others would certainly be implementationally possible, but annoying.

What is being proposed (making JNI weak references have the same
strength as WeakReference) is also a spec change, so would need a lot
more formality than this PR; at least a CSR, and likely a JEP (because
of the potential wide-spread compatibility impact.)

The right fix for the problem that JNI weak references can be used to
resurrect finalizable objects is to remove finalization.
(Finalization has been denigrated "forever", and was formally
deprecated in JDK 9: https://bugs.openjdk.java.net/browse/JDK-8165641.)

> What do you think?

No, don’t make this change.