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

[Remi Forax]

We should really lock Doug Lea and a GC guy in a room and only release them when they have produce a java.util.EphemeronHashMap (or maybe something which doesn't implement the whole Map contract but only put and get), using a WeakHashMap with the value strongly referencing the key is way too common.

see https://bugs.openjdk.java.net/browse/JDK-6389107
and BTW JavasScript WeakMap uses ephemerons.

Rémi

----- Mail original -----
> De: "Peter Levart" <peter.levart at gmail.com>
> À: "Paul Sandoz" <paul.sandoz at oracle.com>, "Core-Libs-Dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mercredi 9 Novembre 2016 09:13:54
> Objet: Re: 8169425: Values computed by a ClassValue should not strongly	reference the ClassValue

> Hi Paul,
> 
> On 11/09/2016 12:27 AM, Paul Sandoz wrote:
>> Hi,
>>
>> Please review the addition of an api note to ClassValue.computeValue.
>>
>> There is some history behind this issue. Another issue was logged [1] related to
>> Groovy using ClassValue and there being a memory leak with classes/loaders not
>> being GC’ed, but it turned out the problem was with Groovy's explicit retention
>> of computed values in a global set. So i closed that issue down.
>>
>> But, there is an edge case where it’s possible to induce out of memory errors
>> with ClassValue, specifically if the computed value holds onto the
>> corresponding ClassValue instance. I think this is an edge case and does not
>> warrant a change to the ClassValue implementation to support weak refs to
>> computed values which is likely to complicate an already intricate
>> implementation and perturb its performance characteristics.
> 
> Simply referencing the associated computed value through a WeakReference
> would break the ClassValue API. It is expected that the associated value
> is strongly reachable through the Class instance with which it is
> associated. Not being reachable strongly, would cause weakly reachable
> associated value to be GCed prematurely. To fix this problem, one would
> need to implement ClassValue using Ephemeron(s) but Java does not
> (yet;-) have them.
> 
>>
>> So i have opted for an api note. I don’t want to normatively specify this, nor
>> do i want to allude to various implementation details. (One can argue a similar
>> note could be written for ThreadLocal.)
>>
>> Thanks,
>> Paul.
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8136353
>>
>> --- a/src/java.base/share/classes/java/lang/ClassValue.java	Tue Nov 08 12:36:21
>> 2016 -0800
>> +++ b/src/java.base/share/classes/java/lang/ClassValue.java	Tue Nov 08 15:25:04
>> 2016 -0800
>> @@ -62,6 +62,13 @@
>>        * If this method throws an exception, the corresponding call to {@code get}
>>        * will terminate abnormally with that exception, and no class value will be
>>        recorded.
>>        *
>> +     * @apiNote
>> +     * Care should be taken to ensure that this {@code ClassValue} is not
>> +     * <a href="../ref/package-summary.html#reachability"><em>strongly
>> reachable</em></a>
>> +     * from the computed value.  Doing so may prevent classes and their loaders
>> +     * from being garbage collected which in turn may induce out of memory
>> +     * errors.
>> +     *
>>        * @param type the type whose class value must be computed
>>        * @return the newly computed value associated with this {@code ClassValue}, for
>>        the given class or interface
>>        * @see #get
> 
> It is not always the case that when ClassValue instance is strongly
> reachable from the associated computed value, unloading of classes and
> class loaders is prevented. So using "may" is correct here. Would it
> make sense to describe the situations where ClassValue instance can
> still be strongly reachable from the associated value and not prevent
> classes and their loaders from being GCed?
> 
> Regards, Peter