2

Why my weakRef.Target is still alive on the second shot?

Could it be a bug? If not, where is the error?

Result:

weakRef.Target is alive = True, expected true because inst keep a hold on SomeClass.
weakRef.Target is alive = True, expected false, because there is no more ref on SomeClass.

Code:

public static class DelegateKeeper
    {
        private static ConditionalWeakTable<object, Action> cwtAction = new ConditionalWeakTable<object, Action>();
        public static void KeepAlive(Action action) => cwtAction.Add(action.Target, action);
    }

    public class SomeClass
    {
        public void DoSomething() { }
    }

    public static class GcHelper
    {
        public static void Collect()
        {
            // OK surely overkill but just to make sure. I will reduce it when everyting will be understood.
            GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true);
            GC.WaitForPendingFinalizers();

            GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true);
            GC.WaitForPendingFinalizers();
        }
    }

    SomeClass instanceSomeClass;
    WeakReference<Action> weakRef;

    [TestMethod]
    public void TestLifeOfObject()
    {
        Init();
        GcHelper.Collect();
        Debug.WriteLine($"weakRef.Target is alive = {weakRef.TryGetTarget(out _)}, expected true because inst keep a hold on SomeClass.");

        RemoveLastReferenceOnSomeClass();
        GcHelper.Collect();
        Debug.WriteLine($"weakRef.Target is alive = {weakRef.TryGetTarget(out _)}, expected false, because there is no more ref on SomeClass.");
    }

    private void Init()
    {
        instanceSomeClass = new SomeClass();
        var action = instanceSomeClass.DoSomething;
        weakRef = new WeakReference<Action>(action);
        DelegateKeeper.KeepAlive(action);
    }

    private void RemoveLastReferenceOnSomeClass()
    {
        instanceSomeClass = null;
    }
Evk
  • 98,527
  • 8
  • 141
  • 191
Eric Ouellet
  • 10,996
  • 11
  • 84
  • 119
  • The current answer (added before bounty) already explains it, it seems. Keys are stored using weak references but values are not (nothing in docs guarantees that), and you have reference to instanceSomeClass in delegate implicitly. – Evk Oct 31 '22 at 06:17
  • You are right. There is a hard ref to my action. But it is linked to the key: which does not have anymore reference to it. So both objects become not-rooted and should be garbage collected. That's the idea of a ConditionalWeakTable. But it does not work for a reason I ignore. (It could be that it should pass through the dispatcher or anyting else but that's what I'm looking for). What I do wrong and what I should wait for it to works. If the ConditionalWeakTable is only a Dictionary, TValue> there would never be a good reason for Microsoft to have created it. – Eric Ouellet Oct 31 '22 at 13:48

2 Answers2

4

The documentation is unclear on this, but ConditionalWeakTable does appear to hold strong references to the value being added, even though the key is only held weakly.

Looking through the source code, it makes use of DependentHandle, for which the documentation says:

A DependentHandle value with a given object instance as target will not cause the target to be kept alive if there are no other strong references to it, but it will do so for the dependent object instance as long as the target is alive.

So because you are adding action as the value, and the action lambda itself captures inst, you are therefore making a strong reference to inst.

You can store other objects though, that you don't care if there is a string reference to. See this fiddle where a new object() is stored as the value, and it works correctly. I don't know your use case, so I don't know whether this works for you.

But I will also note that Microsoft state:

Avoid using weak references as an automatic solution to memory management problems. Instead, develop an effective caching policy for handling your application's objects.

Charlieface
  • 52,284
  • 6
  • 19
  • 43
  • I have to digest everything. About the last point (caching...), I'm not sure about that but my current opinion is that when we define global object that has event that we can subscribe, it is a good pratice to make them "WeakEvent" in order to not having to deal with memory leak (rooted object that we can easily forget). I will get back to you while I will have absorbed everything. Thanks a lot! – Eric Ouellet Oct 27 '22 at 16:00
  • Possibly, although some would argue it's good practice not to have global events in the first place. – Charlieface Oct 27 '22 at 19:12
  • To which I would answer "mmmmm". Say you have a GUI (local app) which want to show any logitem that had been added which is an exception. How would you subscribe to the (global/singleton) log? ... for example – Eric Ouellet Oct 28 '22 at 14:36
  • Your fiddle produces `false` in both cases by the way (maybe because `action` goes out of scope when `Proc1Scoped` ends) – Evk Oct 31 '22 at 16:52
  • Truth is, that is what it should do, because there is nothing else keeping it alive. The `WeakReference` and the `ConditionalWeakTable` do not hold strong references. OP wrote "expected true" but I see no reason why that should be so. I just copied their code. – Charlieface Oct 31 '22 at 18:54
  • But on the other hand source code you linked has this comment: "Inserting a key and value into the dictonary will not prevent the key from dying, even if the key is strongly reachable from the value". If so then it should die in OP case too. – Evk Oct 31 '22 at 20:23
  • @Charlieface, I just want to do 2 things: 1- Bring your attention to the answer I got from the same question asked at Microsoft (which is part of my answer). 2- Thank you very much for your very good answer where I learned some important stuff. But the real answer was really the one from "Viorel-1". My code was suppose to work and it works as expected... if I correct it by removing the first "Debug.WriteLine" which was faulty by taking a hard reference on the target of my weakReference (very hidden). – Eric Ouellet Nov 01 '22 at 14:52
3

I also asked the question at Microsoft and got the right answer from Viorel-1: learn.microsoft.com

Answer: Remove the first "Debug.WriteLine" in sample.

The reason is the exact same one as this question: Unexpected behavior of object life with WeakReference usage [duplicate] and this one: Collect objects still in scope - GC.Collect

The reason is: Either if I use weakReference out parameter discard** : "_", there is a reference to the invisble return value that is used and is in scope. Until that reference stay in scope (the time of the function), there is a reference to the weakReference target. I mean the weakReference stay in state "reachable/rooted" for the garbage collector until the function ends.

By removing the first "Debug.WriteLine", I got the expected behabior. Microsoft reference

** "Discards" are placeholder variables that are intentionally unused in application code. In code, "Discard" is represented as "_" (underscore).

Eric Ouellet
  • 10,996
  • 11
  • 84
  • 119