Hi Charles,
On 03/02/2018 09:22 PM, Charles Oliver Nutter wrote:
Put it another way: does a static reference from a class to itself
prevent that class from being garbage collected? Of course not.
ClassValue is intended to be a way to inject pseudo-static data into
either a class or a Class. Injecting that data, even if it has a
reference back to the class, should not prevent the class from being
collected.
And it should not with ClassValue either. The problem is when you
associate values with "system" classes (such as Object, String, etc.).
Such values become strongly reachable from Object.class, String.class,
etc., respectively. It's like injecting an instance field into Class
class and assigning Object.class.myField = myValue, String.class.myField
= myValue, ... or equivalently injecting static fields into Object,
String, etc class(es) and assigning Object.myField = myValue,
String.myField = myValue, ...
This would not in itself be a problem if myValue was composed only of
objects of system classes (like String, Integer, HashMap, ArrayList,
etc..), but you probably have your own Ruby runtime classes to maintain
the data structure behind myValue. And each of those custom objects
maintains an implicit reference to its runtime class, which is a Ruby
runtime class, loaded by whichever ClassLoader loaded the Ruby runtime,
so the chain of strong references extends from system class loader to
Ruby runtime class loader, effectively preventing it from being unloaded.
If you don't need to use ClassValue.remove() in your code, you could try
using the following wrapper above ClassValue. The overhead of its get()
method compared to ClassValue's should not be to big - just another
lookup into a ThreadLocal and a reference comparison with null (in the
general fast-path case):
/**
* {@link ClassValue} makes mapped values strongly reachable from
{@link Class}(es)
* they are associated with while they are not reachable directly from
{@link ClassValue}(s).
* This is some times desirable, but other times not. This wrapper
provides a
* ClassValue-like API where the roles are reversed - associated values
are not strongly
* reachable from Class(es) but from WeakClassValue(s) they are
associated with.
*
* @param <T> they type of associated values
*/
public abstract class WeakClassValue<T> {
// temporary holder of a Node computed in embedded
cv.computeValue() so we
// can later find out whether the embedded WeakReference has been
installed into
// ClassValueMap or not by comparing it to instance returned from
cv.get()
private static final ThreadLocal<Node> NODE_TL = new ThreadLocal<>();
// head of doubly-linked list of nodes, keeping associated values
reachable from
// WeakClassValue instance
private final Node head = new Node(null, null, null, null, null);
// a single reference queue for all Node(s) to get enqueued when
their referents
// (Class instances) get GCed...
private static final ReferenceQueue<Class<?>> queue = new
ReferenceQueue<>();
// Node is a PhantomReference tracking reachability of associated
Class.
// it also holds the associated value, WeakReference to it, and
prev/next pointers into the list
private static final class Node extends PhantomReference<Class<?>> {
private final Node head;
private Node prev, next;
private Object value;
WeakReference<?> ref;
Node(Class<?> type, ReferenceQueue<Class<?>> queue,
Node head, Object value, WeakReference<?> ref) {
super(type, queue);
this.value = value;
this.ref = ref;
if (head == null) {
// constructing head node
this.prev = this.next = this;
this.head = null;
} else {
// link node into list anchored by head
synchronized (head) {
this.prev = head;
this.next = head.next;
this.head = head; // remember where we belong to
head.next.prev = this;
head.next = this;
}
}
}
// unlink the node from the list using right head as a lock
void unlink() {
synchronized (head) {
prev.next = next;
next.prev = prev;
next = prev = null;
value = null;
ref.clear();
ref = null;
}
}
}
// work is delegated to embedded ClassValue instance
private final ClassValue<WeakReference<T>> cv = new ClassValue<>() {
@SuppressWarnings("unchecked")
@Override
protected WeakReference<T> computeValue(Class<?> type) {
T value = WeakClassValue.this.computeValue(type);
WeakReference<T> ref = new WeakReference<>(value);
// link the value into a doubly-linked list rooted at
// WeakClassValue instance so it is kept alive
Node node = new Node(type, queue, head, value, ref);
// assign the node temporarily to NODE_TL thread-local so
we can
// check later if embedded WeakReference got installed into
// ClassValueMap or not (depends on concurrent activity)
NODE_TL.set(node);
return ref;
}
};
/**
* Computes the given class's derived value for this {@code
WeakClassValue}.
*/
protected abstract T computeValue(Class<?> type);
/**
* Returns the value for the given class.
* If no value has yet been computed, it is obtained by
* an invocation of the {@link #computeValue computeValue} method.
*/
public T get(Class<?> type) {
WeakReference<T> ref = cv.get(type);
Node node = NODE_TL.get();
if (node != null) {
if (ref != node.ref) {
// some other thread has beaten us installing its
WeakReference
// into type's ClassValueMap. We must unlink our Node
from list
// to release our stale value
node.unlink();
}
NODE_TL.set(null);
}
try {
return ref.get();
} finally {
// keep type and this reachable as those are guarantees that
// the value is reachable too before it is get() from 'ref'
Reference.reachabilityFence(type);
Reference.reachabilityFence(this);
}
}
/**
* Removes any stale nodes from doubly-linked list(s) holding
values that
* were associated with classes that have been GCed. You cal call this
* explicitly to expedite removal of stale data if new values are not
* computed for some time
*/
public static void removeStaleValues() {
Node n;
while ((n = (Node) queue.poll()) != null) {
n.unlink();
}
}
}
You may freely use this code in Ruby or anywhere else if you find fit.
Regards, Peter
On Fri, Mar 2, 2018 at 2:19 PM Charles Oliver Nutter
<head...@headius.com <mailto:head...@headius.com>> wrote:
I have posted a modified version of my description to the main bug
report.
TLDR: ClassValue should not root objects.
- Charlie
On Fri, Mar 2, 2018 at 2:13 PM Charles Oliver Nutter
<head...@headius.com <mailto:head...@headius.com>> wrote:
Yes, it may be the same bug.
In my case, the ClassValue is held by a utility object used
for our Java integration. That utility object has to live
somewhere, so it's held by the JRuby runtime instance. There's
a strong reference chain leading to the ClassValue.
The value is a Ruby representation of the class, with
reflected methods parsed out and turned into Ruby endpoints.
Obviously, the value also references the class, either
directly or indirectly through reflected members.
The Ruby class wrapper is only hard referenced directly if
there's an instance of the object live and moving through
JRuby. It may be referenced indirectly through inline caches.
However...I do not believe this should prevent collection of
the class associated with the ClassValue.
The value referenced in the ClassValue should not constitute a
hard reference. If it is alive *only* because of its associate
with a given class, that should not be enough to root either
the object or the class.
ClassValue should work like ThreadLocal. If the Thread
associated with a value goes away, the value reference goes
away. ThreadLocal does nothing prevent it from being
collected. If the Class associated with a Value goes away, the
same should happen to that Value and it should be collectable
once all other hard references are gone.
Perhaps I've misunderstood?
- Charlie
On Fri, Mar 2, 2018 at 12:16 PM Vladimir Ivanov
<vladimir.x.iva...@oracle.com
<mailto:vladimir.x.iva...@oracle.com>> wrote:
Charlie,
Does it look similar to the following bugs?
https://bugs.openjdk.java.net/browse/JDK-8136353
https://bugs.openjdk.java.net/browse/JDK-8169425
If that's the same (and it seems so to me [1]), then speak
up and
persuade Paul it's an important edge case (as stated in
JDK-8169425).
Best regards,
Vladimir Ivanov
[1] new RubyClass(Ruby.this) in
public static class Ruby {
private ClassValue<RubyClass> cache = new
ClassValue<RubyClass>() {
protected RubyClass computeValue(Class<?> type) {
return new RubyClass(Ruby.this);
}
};
On 3/1/18 2:25 AM, Charles Oliver Nutter wrote:
> So I don't think we ever closed the loop here. Did
anyone on the JDK
> side confirm this, file an issue, or fix it?
>
> We still have ClassValue disabled in JRuby because of
the rooting issues
> described here and in
https://github.com/jruby/jruby/pull/3228.
>
> - Charlie
>
> On Thu, Aug 27, 2015 at 7:04 AM Jochen Theodorou
<blackd...@gmx.org <mailto:blackd...@gmx.org>
> <mailto:blackd...@gmx.org <mailto:blackd...@gmx.org>>>
wrote:
>
> One more thing...
>
> Remi, I tried your link with my simplified scenario
and it does there
> not stop the collection of the classloader
>
> Am 27.08.2015 11:54, schrieb Jochen Theodorou:
> > Hi,
> >
> > In trying to reproduce the problem outside of
Groovy I stumbled
> over a
> > case case which I think should work
> >
> > public class MyClassValue extends ClassValue {
> > protected Object computeValue(Class type) {
> > Dummy ret = new Dummy();
> > Dummy.l.add (this);
> > return ret;
> > }
> > }
> >
> > class Dummy {
> > static final ArrayList l = new ArrayList();
> > }
> >
> > basically this means there will be a hard
reference on the ClassValue
> > somewhere. It can be in a static or non-static
field, direct or
> > indirect. But this won't collect. If I put for
example a
> WeakReference
> > in between it works again.
> >
> > Finally I also tested to put the hard reference
in a third class
> > instead, to avoid this self reference. But it can
still not collect.
> >
> > So I currently have the impression that if
anything holds a hard
> > reference on the class value that the classloader
cannot be collected
> > anymore.
> >
> > Unless I misunderstand something here I see that
as a bug
> >
> > bye blackdrag
> >
>
>
> --
> Jochen "blackdrag" Theodorou
> blog: http://blackdragsview.blogspot.com/
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev@openjdk.java.net
<mailto:mlvm-dev@openjdk.java.net>
<mailto:mlvm-dev@openjdk.java.net
<mailto:mlvm-dev@openjdk.java.net>>
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>
> --
>
> - Charlie (mobile)
>
>
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev@openjdk.java.net <mailto:mlvm-dev@openjdk.java.net>
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>
--
- Charlie (mobile)
--
- Charlie (mobile)
--
- Charlie (mobile)
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
