8169425: Values computed by a ClassValue should not strongly reference the ClassValue

[Paul Sandoz]

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.

Paul.


> 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<T> {
> 
>     /**
>      * 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
>      * <a href="../ref/package-summary.html#reachability"><em>weakly reachable</em></a>
>      * 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.
>      * <p>
>      * 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 <T> the type of values associated with created ClassValue instance
>      * @return new instance of ClassValue, weakly referencing given factory function.
>      * @since 9
>      */
>     public static <T> ClassValue<T> withWeakFactory(
>         Function<? super Class<?>, T> factory)
>     {
>         WeakReference<Function<? super Class<?>, T>> factoryRef =
>             new WeakReference<>(factory);
> 
>         return new ClassValue<T>() {
>             @Override
>             protected T computeValue(Class<?> type) {
>                 Function<? super Class<?>, 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<T> extends ClassValue<T> {
>>     private final WeakReference<Function<? super Class<?>, ? extends T>> factoryRef;
>> 
>>     public WeakFactoryClassValue(Function<? super Class<?>, ? extends T> factory) {
>>         factoryRef = new WeakReference<>(factory);
>>     }
>> 
>>     @Override
>>     protected T computeValue(Class<?> type) {
>>         Function<? super Class<?>, ? 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 VALUE_FACTORY stay at least until MyApp class is alive
>>     private static final Function<Class<?>, Object> VALUE_FACTORY = clazz -> MyApp.CV;
>> 
>>     public static final ClassValue<Object> CV =
>>         new WeakFactoryClassValue<>(VALUE_FACTORY);
>> 
>>     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 VALUE_FACTORY 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<T> extends ClassValue<T> {
>> > private final WeakReference<Supplier<T>> supplierRef;
>> >
>> > public WeakSupplierClassValue(Supplier<T> supplier) { supplierRef =
>> > new WeakReference<>(supplier); }
>> >
>> > @Override protected T computeValue(Class<?> type) { Supplier<T>
>> > 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 CV_SUPPLIER stay at least until MyApp
>> > class is alive private static final Supplier<Object> CV_SUPPLIER = ()
>> > -> MyApp.CV;
>> >
>> > public static final ClassValue<Object> CV = new
>> > WeakSupplierClassValue<>(CV_SUPPLIER);
>> >
>> > 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
>> > CV_SUPPLIER is only weakly reachable CV.get(Object.class); } }
>> >
>> >
>> > Regards, Peter
>> 
>