2
class Program
{
    static void Main(string[] args)
    {
        var inst = new SomeClass();
        var weakRef = new WeakReference<Action>(inst.DoSomething);
        GC.Collect();
        Console.WriteLine($"inst is alive = {inst != null} : weakRef.Target is alive = {weakRef.TryGetTarget(out Action callback)}");
        Console.ReadLine();
    }
}

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

The output shows that inst is not null, but the reference that WeakReference<Action> points to is. I expect this is because a new Action is created that points to the instance method, rather than storing a reference to the instance's method itself.

How can I hold a weak reference to a method of an object instance for the duration that the instance is not yet garbage collected?

Peter Morris
  • 20,174
  • 9
  • 81
  • 146
  • You are holding the `inst` reference, but noone is holding the auto generated `Action` object by the syntactic sugar you wrote. Try it by explicit using a variable of type `Action`. – Progman Jul 21 '18 at 14:21
  • @Progman I realised the problem and changed the question just before you answered. – Peter Morris Jul 21 '18 at 14:53
  • What happens when you save the Action in a variable before the `GC.Collect();` call? It shouldn't be claimed when you have an active reference to the Action object. – Progman Jul 21 '18 at 15:16
  • Save `Action` to the `SomeClass` field. That way `Action` can not be collected before `SomeClass`. If modifying `SomeClass` is not possible, then you can use [`ConditionalWeakTable`](https://learn.microsoft.com/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2) class. – user4003407 Jul 21 '18 at 15:20
  • @PetSerAl You should post that as an answer. – Peter Morris Jul 21 '18 at 15:30

2 Answers2

5

If you need that Action instance would not be collected before SomeClass instance, then you need to add reference from SomeClass instance to Action instance. It can be instance field of SomeClass that point to Action instance, but if you can not alter SomeClass definition, than you can use ConditionalWeakTable<TKey,TValue> class to attach field dynamically.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;
[assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default)]
public class SomeClass {
    public void DoSomething() { }
}
public static class DelegateKeeper {
    private static ConditionalWeakTable<object, List<Delegate>> cwt = new ConditionalWeakTable<object, List<Delegate>>();
    public static void KeepAlive(Delegate d) => cwt.GetOrCreateValue(d?.Target ?? throw new ArgumentNullException(nameof(d))).Add(d);
}
static class Program {
    static void Main() {
        SomeClass inst = new SomeClass();
        Action a1 = inst.DoSomething;
        DelegateKeeper.KeepAlive(a1);
        Action a2 = inst.DoSomething;
        WeakReference<SomeClass> winst = new WeakReference<SomeClass>(inst);
        WeakReference<Action> wa1 = new WeakReference<Action>(a1);
        WeakReference<Action> wa2 = new WeakReference<Action>(a2);
        GC.Collect();
        Console.WriteLine($"{winst.TryGetTarget(out _),5}:{wa1.TryGetTarget(out _),5}:{wa2.TryGetTarget(out _),5}");
        GC.KeepAlive(a1);
        GC.KeepAlive(a2);
        GC.Collect();
        Console.WriteLine($"{winst.TryGetTarget(out _),5}:{wa1.TryGetTarget(out _),5}:{wa2.TryGetTarget(out _),5}");
        GC.KeepAlive(inst);
        GC.Collect();
        Console.WriteLine($"{winst.TryGetTarget(out _),5}:{wa1.TryGetTarget(out _),5}:{wa2.TryGetTarget(out _),5}");
    }
}

Output:

 True: True: True
 True: True:False
False:False:False

tio.run

In DelegateKeeper class I use List<Delegate> as dependent object type, so you can keep multiple delegates per one class instance. I use Delegate.Target as key to the table, so you do not need to pass instance separately. This would not work with anonymous methods, since them likely to have compiler generated closure class in Target property. GetOrCreateValue get value bound to key or create new one using default constructor and add it to table automatically.

user4003407
  • 21,204
  • 4
  • 50
  • 60
  • I've got to admit, it took me *way* longer than I expected to understand what is going on with `DelegateKeeper`. I think I've finally got it now, but would you mind explaining it in a bit more detail? I think it will make this a much better answer. – Bradley Uffner Jul 22 '18 at 15:52
  • Wait, after thinking about this more, and digging more in to the MSDN link, don't you need to add the result of `cwt.GetOrCreateValue` to `cwt` with `cwt.Add`? `GetOrCreateValue` doesn't say that it automatically adds and tracks the value by the key. This existing `Add` that you have just adds the delegate to the `List`, not `cwt`. – Bradley Uffner Jul 22 '18 at 16:07
  • @BradleyUffner [Docs](https://learn.microsoft.com/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2.getorcreatevalue): If the key does not exist in the table, the method invokes the default constructor of the class that represents the table's value to create **a value that is bound to the specified key.** – user4003407 Jul 22 '18 at 17:15
  • Source: [`GetOrCreateValue`](https://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/ConditionalWeakTable.cs,274): Helper method to call GetValue without passing a creation delegate. [`GetValue`](https://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/ConditionalWeakTable.cs,214): Atomically tests if key exists in table. If so, returns corresponding value. If not, invokes createValueCallback() passing it the key. **The returned value is bound to the key in the table** and returned as the result of GetValue(). – user4003407 Jul 22 '18 at 17:15
  • I think your example code overcomplicates your answer, but `ConditionalWeakTable` looks like what I was after. Thank you! – Peter Morris Jul 22 '18 at 18:55
4

Creating an Action instance from a MethodGroup creates an instance that isn't attached in any way to the instance of the class that the method is a member of. That means that there is nothing keeping your Action rooted, and the GC will happily collect it.

If you store a reference to the Action as a member of the class, it will become rooted to the lifetime of that instance, preventing it from being collected as long as that instance is alive.

public class SomeClass
{
    public SomeClass()
    {
        DoSomething = this.DoSomething_Internal ;
    }

    public Action DoSomething { get; } 

    private void DoSomething_Internal() { }
}

Note: I made the original DoSomething private, and renamed it to DoSomething_Internal, replacing the old DoSomething with a readonly property in order to keep the class signature as close as possible to your original class. You don't need to follow this pattern exactly, any reference to the Action stored in the class, including in a normal field, will do. Though you will still have to expose that reference somehow if you actually want to be able to use it.

You can test it like this:

var inst = new SomeClass();
var weakRef = new WeakReference<Action>(inst.DoSomething);

GC.Collect();
GC.WaitForPendingFinalizers(); // You should do this after forcing a GC, just in case there is still GC work being done in the background.

Console.WriteLine($"inst is alive = {inst != null} : weakRef.Target is alive = {weakRef.TryGetTarget(out Action callback1)}");

// These next 2 lines discard local variables, as Hans points out in the comments,  
// DO NOT do this in production code.  Please read the link he posted for more details.
inst = null; // discard the class instance
callback1 = null; // discard the temporary Action instance from TryGetTarget, otherwise it will act as a GC Root, preventing it from being collected later.

GC.Collect();
GC.WaitForPendingFinalizers();

Console.WriteLine($"inst is alive = {inst != null} : weakRef.Target is alive = {weakRef.TryGetTarget(out Action callback2)}");

Which produces the following output:

inst is alive = True : weakRef.Target is alive = True
inst is alive = False : weakRef.Target is alive = False

Bradley Uffner
  • 16,641
  • 3
  • 39
  • 76
  • 1
    Don't set these object references to null please. https://stackoverflow.com/a/17131389/17034 – Hans Passant Jul 21 '18 at 16:13
  • 1
    @HansPassant That's some fascinating insight in to the GC, thank you! I was using the test code in LINQPad, and setting those instances to `null` seemed the easiest way to prove that the `Action` was collected when the class instance was. I would never advocate using that pattern in "real" code. Though that does make me realize that I should probably be calling `GC.WaitForPendingFinalizers` after `GC.Collect`, just for extra safety. – Bradley Uffner Jul 21 '18 at 16:20
  • I need to be able to implement something that does not require me to alter `SomeClass` – Peter Morris Jul 21 '18 at 20:56