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

[Doug Lea]

On 11/09/2016 03:32 AM, Remi Forax wrote:
> 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.
>

Well, GC is an internal API for several collectors, not a specific
mechanism, so you'd need a bunch of GC engineers. In the near term,
it's more productive to identify specific problems and issues, as
Peter has been doing.

And: Similar cases can indeed occur with ThreadLocal, but users seem
to mostly avoid them, sometimes after painful experience.

-Doug


> 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
>