I had some similar concerns with some data structures I maintain in Dynalink; I 
specifically have a utility method to check if classes from two class loaders 
are allowed to keep strong references to each other 
(jdk.dynalink.internal.InternalTypeUtilities.canReferenceDirectly if you want 
to look it up). It’s used in few places to either choose between strong 
references and weak references, or to forgo some caching for unrelated classes 
entirely lest it creates an unwanted strong reference between class loaders.

Attila.

> On 2018. Mar 28., at 14:42, Peter Levart <peter.lev...@gmail.com> wrote:
> 
> 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> 
>> 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> 
>> 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> 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>> 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>
>> >     http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>> >
>> > --
>> >
>> > - Charlie (mobile)
>> >
>> >
>> >
>> > _______________________________________________
>> > mlvm-dev mailing list
>> > 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

_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Reply via email to