c# - Strange behavior with actions, local variables and garbage collection in MVVM light Messenger

Question

I am having a really strange problem with the Messenger system in MVVM Light. It's hard to explain, so here is small program that demonstrates the issue:

using System;
using GalaSoft.MvvmLight.Messaging;

namespace TestApp
{
    class Program
    {
        static void Main(string[] args)
        {
            var prog = new Program();
            var recipient = new object();

            prog.RegisterMessageA(recipient);
            prog.RegisterMessageB(recipient);

            prog.SendMessage("First Message");
            GC.Collect();
            prog.SendMessage("Second Message");
        }

        public void RegisterMessageA(object target)
        {
            Messenger.Default.Register(this, (Message msg) =>
            {
                Console.WriteLine(msg.Name + " recieved by A");
                var x = target;
            });
        }

        public void RegisterMessageB(object target)
        {
            Messenger.Default.Register(this, (Message msg) =>
            {
                Console.WriteLine(msg.Name + " received by B");
            });
        }

        public void SendMessage(string name)
        {
            Messenger.Default.Send(new Message { Name = name });
        }

        class Message
        {
            public string Name { get; set; }
        }
    }
}

If you run the application, this is the console output:

First Message recieved by A
First Message received by B
Second Message received by B

As you can see, the second message is never received by recipient A. However, the only difference between B and A is one line: the statement var x = target;. If you remove this line, A receives the second message.

Also, if you remove GC.Collect(); then A receives the second message. However, this only hides the issue, as in a real program the garbage collector will automatically run eventually.

Why is this happening? I assume that somehow, if the recipient action refers to a variable from it's containing method scope, it ties the action's lifetime to that scope so that once out of the scope it can be garbage collected. I don't understand why this is at all. I also don't understand why actions that do not reference variables from the scope they are defined in do not have this problem.

Can anyone explain what is going on here?

Answer 1

Well, I now understand why it's happening (I believe, anyway). I've reproduced it in a shorter form which doesn't use lambda expressions, and then I'll explain why the lambdas are important.

using System;
using GalaSoft.MvvmLight.Messaging;

class Program
{
    static void Main(string[] args)
    {
        Receiver r1 = new Receiver("r1");
        Receiver r2 = new Receiver("r2");
        var recipient = new object();

        Messenger.Default.Register<object>(recipient, r1).ShowMessage;
        Messenger.Default.Register<object>(recipient, r2).ShowMessage;

        GC.Collect();
        Messenger.Default.Send(recipient, null);
        // Uncomment one of these to see the relevant message...
        // GC.KeepAlive(r1);
        // GC.KeepAlive(r2);
    }
}

class Receiver
{
    private string name;

    public Receiver(string name)
    {
        this.name = name;
    }

    public void ShowMessage(object message)
    {
        Console.WriteLine("message received by {0}", name);
    }
}

Basically, the messenger only keeps a weak reference to the message handler. (Also to the recipient, but that's not a problem here.) More specifically, it appears to have a weak reference to the handler's target object. It doesn't seem to care about the delegate object itself, but the target is important. So in the above code, when you keep a Receiver object alive, the delegate which has that object as a target is still used. However, when the target is allowed to be garbage collected, the handler using that object is not used.

Now let's look at your two handler:

public void RegisterMessageA(object target)
{
    Messenger.Default.Register(target, (Message msg) =>
    {
        Console.WriteLine(msg.Name + " received by A");
        var x = target;
    });
}

This lambda expression captures the target parameter. In order to capture it, the compiler generates a new class - so RegisterMessageA is effectively:

public void RegisterMessageA(object target)
{
    GeneratedClass x = new GeneratedClass();
    x.target = target;
    Messenger.Default.Register(x.target, x.Method);
}

private class GeneratedClass
{
    public object target;

    public void Method(Message msg)
    {
        Console.WriteLine(msg.Name + " received by A");
        var x = target;
    }
}

Now, there's nothing other than the delegate which keeps that instance of GeneratedClass alive. Compare that with your second handler:

public void RegisterMessageB(object target)
{
    Messenger.Default.Register(target, (Message msg) =>
    {
        Console.WriteLine(msg.Name + " received by B");
    });
}

Here, there are no captured variables, so the compiler generates code a bit like this:

public void RegisterMessageB(object target)
{
    Messenger.Default.Register(target, RegisterMessageB_Lambda);
}

private static void RegisterMessageB_Lambda(Message msg)
{
    Console.WriteLine(msg.Name + " received by B");
}

Here it's a static method, so there's no delegate target at all. If the delegate captured this, it would be generated as an instance method. But the important point is that there's no need to generate an extra class... so there's nothing to be garbage collected.

I haven't looked into exactly how MvvmLight is doing this - whether it's just got a weak reference to the delegate, and that the CLR is treating that in some special way, or whether MvvmLight is separating the target from the delegate itself. Either way, I hope that explains the behaviour you're seeing. In terms of how to fix whatever problem you're seeing with real code - basically you'll need to make sure you keep a strong reference to whatever delegate target you need.

EDIT: Okay, it looks like it's now due to WeakActionGeneric and its base class WeakAction. I don't know whether this behaviour is the expected behaviour (by the author), but that's the code responsible :)