Hm,
Such advise might not be fruitful for average user. Why?
1. ClassValue instances are typically assigned to static final fields of
application classes. Which means that they will be strongly reachable as
long as the ClassLoader of such static field's declaring class is
strongly reachable. In case the app is deployed in some app server, this
will be a non-permanent class loader (supporting undeployment of the app).
2. ClassValue associated values (i.e. computed values) are typically
composed of objects that are instances of application classes too. We
can't expect users to only use objects that are instances of system
classes (HashMap, String, Integer, etc... loaded by some "permanent"
class loader such as bootstrap or platform CL).
How can now user adhere to the @implNote?
Even if the user pays attention and does not directly reference the
ClassValue instance from the associated value(s) through some explicit
chain of fields, the ClassValue instance will be strongly reachable from
the associated value:
associated class (the argument to ClassValue.get()) -> associated value
(i.e. computed value) -> associated value class (presumably some app
class) -> app class loader -> all app classes (including static field's
declaring class) -> ClassValue instance
if the associated class is some system class (String, Integer, etc...),
all above instances are permanently strongly reachable!
Should @implNote advise against either assigning ClassValue instance to
static fields of application classes or maybe against using application
classes to construct computed values?
Just using ClassValue<WeakReference<AppData>> is not a complete solution
since AppData instances must also be strongly reachable from app class
loader or they will be GCed prematurely.
We could advise that instead of:
public class AppDataHolder {
public static final ClassValue<AppData> APP_DATA_CV = new
ClassValue<>() {
@Override
protected AppData computeValue(Class<?> type) {
return new AppData();
}
};
Users do something like the following:
public class AppDataHolder {
// make AppData instances strongly reachable from app class loader
private static final Queue<AppData> _APP_DATAS = new
ConcurrentLinkedQueue<>();
private static final ClassValue<WeakReference<AppData>>
_WEAK_APP_DATA_CV = new ClassValue<>() {
@Override
protected WeakReference<AppData> computeValue(Class<?> type) {
AppData appData = new AppData();
// make appData strongly reachable from the app class loader
_APP_DATAS.add(appData);
return new WeakReference<>(appData);
}
};
// use this function...
public static final Function<Class<?>, AppData> APP_DATA_FN =
clazz ->
Objects.requireNonNull(_WEAK_APP_DATA_CV.get(clazz).get());
Regards, Peter
On 05/10/2017 08:16 PM, Paul Sandoz wrote:
Hi,
I would like to still propose this note for 9, we ain’t gonna resolve the
underlying issue in 9.
It’s currently an api note but it’s more appropriate as an implementation note
(e.g. Ephemerons might help).
We could amend the note with Peter’s suggestion as follows:
@implNote
Care should be taken to ensure that this ClassValue is not strongly reachable
from the computed value, more specifically for the case when the class loader
of the computed value’s class is not the same as or a descendent of the class
loader of this ClassValue’s class.
Doing so will prevent classes and their loader from being garbage collected
which in turn may induce out of memory errors.
Paul.
On 8 Nov 2016, at 15:27, Paul Sandoz <paul.san...@oracle.com> 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.
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
