8169425: Values computed by a ClassValue should not strongly reference the ClassValue (original) (raw)

Peter Levart peter.levart at gmail.com
Wed Nov 9 18:56:28 UTC 2016

Hi Remi,

On 11/09/2016 06:29 PM, Remi Forax wrote:

----- Mail original -----

De: "Paul Sandoz" <paul.sandoz at oracle.com> Cc: "Core-Libs-Dev" <core-libs-dev at openjdk.java.net> Envoyé: Mercredi 9 Novembre 2016 17:47:34 Objet: Re: 8169425: Values computed by a ClassValue should not strongly reference the ClassValue Hi Peter,

Good point about if such support was added it would break the API and also (with Remi) about Ephemerons. You are correct in stating the constraints in more detail regarding classes in the same loader. However, i think that is going into more detail than I would prefer for what i think is an edge case. So I want in general to warn developers away from strongly referencing this ClassValue in the computed value for any associated class. If we do get strong feedback that this is a real problem we could consider adding a clever little static method like you suggest, with caveats that the computing Function might go away. At the moment I would prefer to keep things simple and say as little as possible. I agree with Paul, given the number of people that use ClassValue, i think we should keep the thing simple and let people write their own class on top of ClassValue if they need it.

That's OK. I don't think such class or static method should be added if there is no demand for it. But the point I was trying to make is that while people can write such utility class themselves, in order to be effective, they would have to deploy it with a class loader that is otherwise permanent (that doesn't need to go away) and such that the class is visible to application. This means that they can't just bundle such class with the application.

Regards, Peter

Paul. Rémi

On 9 Nov 2016, at 05:15, Peter Levart <peter.levart at gmail.com> wrote:

Hi Paul, What do you think of introducing a static factory method in ClassValue in addition to your @implNotes. The method would go like this (similar to ThreadLocal.withInitial()): public abstract class ClassValue { /** * Creates a {@code ClassValue} instance which uses given {@code factory} * function for computing values associated with classes passed as arguments * to {@link #get} method. The given {@code factory} function will only be * weakly reachable * from the created ClassValue instance, so one must ensure that is is not Garbage * Collected at least until the returned ClassValue is not used any more. *

* Attempts to use created ClassValue instance to lazily calculate another * associated value after the given factory function is GCed will result in * {@link IllegalStateException} being thrown from the {@link #get} method. * * @param factory the function to be used to produce values associated with * passed-in classes and created ClassValue instance * @param the type of values associated with created ClassValue instance * @return new instance of ClassValue, weakly referencing given factory function. * @since 9 */ public static ClassValue withWeakFactory( Function, T> factory) { WeakReference<Function, T>> factoryRef = new WeakReference<>(factory); return new ClassValue() { @Override protected T computeValue(Class<?> type) { Function, T> factory = factoryRef.get(); if (factory == null) { throw new IllegalStateException( "The value factory function has already been GC(ed)."); } return factory.apply(type); } }; } @implNotes could point to this method with an example...

Regards, Peter On 11/09/2016 01:49 PM, Peter Levart wrote: Or, better yet, using value factory Function instead of Supplier:

public class WeakFactoryClassValue extends ClassValue { private final WeakReference<Function, ? extends T>> factoryRef; public WeakFactoryClassValue(Function, ? extends T> factory) { factoryRef = new WeakReference<>(factory); } @Override protected T computeValue(Class<?> type) { Function, ? extends T> factory = factoryRef.get(); if (factory == null) { throw new IllegalStateException("Value factory function has already been GCed"); } return factory.apply(type); } } The example would then read: public class MyApp { // make VALUEFACTORY stay at least until MyApp class is alive private static final Function<Class<?>, Object> VALUEFACTORY = clazz -> MyApp.CV; public static final ClassValue CV = new WeakFactoryClassValue<>(VALUEFACTORY); public static void main(String[] args) { // this is OK CV.get(MyApp.class); // even this is OK, it makes CV reachable from Object.class, // but VALUEFACTORY is only weakly reachable CV.get(Object.class); } }

Regards, Peter On 11/09/2016 01:31 PM, Peter Levart wrote: The above situation could be prevented by a special concrete ClassValue implementation, provided by the platform (loaded by bootstrap CL): public class WeakSupplierClassValue extends ClassValue { private final WeakReference<Supplier> supplierRef; public WeakSupplierClassValue(Supplier supplier) { supplierRef = new WeakReference<>(supplier); } @Override protected T computeValue(Class<?> type) { Supplier supplier = supplierRef.get(); if (supplier == null) { throw new IllegalStateException("Supplier has already been GCed"); } return supplier.get(); } } ...with such utility class, one could rewrite above example to: public class MyApp { // make CVSUPPLIER stay at least until MyApp class is alive private static final Supplier CVSUPPLIER = () -> MyApp.CV; public static final ClassValue CV = new WeakSupplierClassValue<>(CVSUPPLIER); public static void main(String[] args) { // this is OK CV.get(MyApp.class); // even this is OK, it makes CV reachable from Object.class, // but CVSUPPLIER is only weakly reachable CV.get(Object.class); } } Regards, Peter

More information about the core-libs-dev mailing list