Why and How to Avoid Event Handler Memory Leaks

Why and How to avoid Event Handler memory leaks?

The cause is simple to explain: while an event handler is subscribed, the publisher of the event holds a reference to the subscriber via the event handler delegate (assuming the delegate is an instance method).

If the publisher lives longer than the subscriber, then it will keep the subscriber alive even when there are no other references to the subscriber.

If you unsubscribe from the event with an equal handler, then yes, that will remove the handler and the possible leak. However, in my experience this is rarely actually a problem - because typically I find that the publisher and subscriber have roughly equal lifetimes anyway.

It is a possible cause... but in my experience it's rather over-hyped. Your mileage may vary, of course... you just need to be careful.

How do events cause memory leaks in C# and how do Weak References help mitigate that?

When a listener attaches an event listener to an event, the source object will get a reference to the listener object. This means that the listener cannot be collected by the garbage collector until either the event handler is detached, or the source object is collected.

Consider the following classes:

class Source
{
    public event EventHandler SomeEvent;
}

class Listener
{
    public Listener(Source source)
    {
        // attach an event listner; this adds a reference to the
        // source_SomeEvent method in this instance to the invocation list
        // of SomeEvent in source
        source.SomeEvent += new EventHandler(source_SomeEvent);
    }

    void source_SomeEvent(object sender, EventArgs e)
    {
        // whatever
    }
}

...and then the following code:

Source newSource = new Source();
Listener listener = new Listener(newSource);
listener = null;

Even though we assign null to listener, it will not be eligible for garbage collection, since newSource is still holding a reference to the event handler (Listener.source_SomeEvent). To fix this kind of leak, it is important to always detach event listeners when they are no longer needed.

The above sample is written to focus on the problem with the leak. In order to fix that code, the easiest will perhaps be to let Listener hold on to a reference to Source, so that it can later detach the event listener:

class Listener
{
    private Source _source;
    public Listener(Source source)
    {
        _source = source;
        // attach an event listner; this adds a reference to the
        // source_SomeEvent method in this instance to the invocation list
        // of SomeEvent in source
        _source.SomeEvent += source_SomeEvent;
    }

    void source_SomeEvent(object sender, EventArgs e)
    {
        // whatever
    }

    public void Close()
    {
        if (_source != null)
        {
            // detach event handler
            _source.SomeEvent -= source_SomeEvent;
            _source = null;
        }
    }
}

Then the calling code can signal that it is done using the object, which will remove the reference that Source has to ´Listener`;

Source newSource = new Source();
Listener listener = new Listener(newSource);
// use listener
listener.Close();
listener = null;

What kind of memory leaks do lambda event handlers cause?

Disclaimer:
I can't guarantee this is 100% truth - your question is quite deep and I could make a mistake.

However, I hope it will give you some thoughts or directions.

Let's consider this question according CLR memory organization:

Local method variables and method parameters are stored in the method stack frame in memory (except they declared with ref keyword).

Stack stores value types and reference-type variables references which point on objects in heap.

Method stack frame is exists while method execution, and local method variables will dissappear with stack frame after method ended.

Except if local variables were captured one way or another, it also relates to compiler work, you can read about it at Jon Skeet's website:

http://jonskeet.uk/csharp/csharp2/delegates.html#captured.variables

Version 1: OnSomeEvent method is member of MyClass and it will captured by Someclass source instance, until delegates that refers on this method will not be removed from event. So, MyClass instance that was created in constructor, placed in heap and holds this method will not be collected by GC until its method reference will not be removed from event.

Compiler compiles lambda by specific way, please read Implementation example paragraph fully:

https://github.com/dotnet/csharplang/blob/master/spec/conversions.md#anonymous-function-conversions

Version 4:
2 links I provided give a kick lambda will be compiled to MyClass method, which will be captured by SomeClass instance as in Version 1

Version 2:
I don't know nuances about how local methods will be compiled, but it should be same as in Version 4 (and, therefore, Version 1).

Version 3:
All local variables will be captured by interesting way.

You have 'object x' also, so compiler generated class will be created, which
will contain public field public object x; and method which will be translated from your lambda (see Implementation example paragraph).

So, I think in Versions 1,2,4 will be internally the same:
MyClass will contain the method which will be used as event handler.

In Version 3 compiler generated class will be created and it will hold your local variable and method translated from lamdba.

Any instance of any class will not be collected by GC until SomeClass event has its method in invocation list.

I think "magic helper object" you meant is compiler generated class instance.
You can remove subscription by assigning null to event: source.SomeEvent = null.
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/events/how-to-subscribe-to-and-unsubscribe-from-events#unsubscribing

C# Events Memory Leak

Let's say that A references B. Furthermore, say you think you're done with B and expect it to be garbage collected.

Now, if A is reachable[1], B won't be garbage collected, despite the fact that "you're done with it". This is, in all essence, a memory leak[2]

If B subscribes to an event in A, then we have the same situation: A has a reference to B via the event handler delegate.

So, when is this a problem? Only when the referencing object is reachable, as mentioned above. In this case, there can be a leak when a Foo instance isn't used any longer:

class Foo
{
    Bar _bar;

    public Foo(Bar bar)
    {
        _bar = bar;
        _bar.Changed += BarChanged;
    }

    void BarChanged(object sender, EventArgs e) { }
}

The reason why there can be a leak is that the Bar instance passed in the constructor can have a longer lifetime than the Foo instance using it. The subscribed event handler can then keep the Foo alive.

In this case you need to provide a way to unsubscribe from the event to not get a memory leak. One way of doing that is by letting Foo implement IDisposable. The upside of that is that it clearly signals to the class consumer that he need to call Dispose() when done. Another way is to have separate Subscribe() and Unsubscribe() methods, but that doesn't convey the type's expectations - they are too optional to call and introduce a temporal coupling.

My recommendation is:

class sealed Foo : IDisposable
{
    readonly Bar _bar;
    bool _disposed;

    ...

    public void Dispose()
    {
        if (!_disposed)
        {
            _disposed = true;
            _bar.Changed -= BarChanged;
        }
    }

    ...
}

Or alternatively:

class sealed Foo : IDisposable
{
    Bar _bar;

    ...

    public void Dispose()
    {
        if (_bar != null)
        {
            _bar.Changed -= BarChanged;
            _bar = null;
        }
    }

    ...
}

On the other hand, when the referencing object isn't reachable, there can't be a leak:

class sealed Foo
{
    Bar _bar;

    public Foo()
    {
        _bar = new Bar();
        _bar.Changed += BarChanged;
    }

    void BarChanged(object sender, EventArgs e) { }
}

In this case any Foo instance will always outlive its composed Bar instance. When a Foo is unreachable, so will its Bar be. The subscribed event handler cannot keep the Foo alive here. The downside of this is that if Bar is a dependency in need of being mocked in unit testing scenarios, it can't (in any clean way) be explicitly instantiated by the consumer, but needs to be injected.

[1] http://msdn.microsoft.com/en-us/magazine/bb985010.aspx

[2] http://en.wikipedia.org/wiki/Memory_leak

Avoiding event memory leak

This will not cause any memory leaks. As you are unsubscribing from the event before removing reference to an object, and then hooking up to the new one when it comes in, this is correct and wont cause a memory leak. As Ivan mentioned you will need to ensure that events are unsubscribed from when disposing the object, there are a couple of ways you can control destruction of the object. either make a destructor by creating a ~(classname) Method or implementing IDisposable if you have other external things to clean up as well. Call Dispose when you are done with the object and you should be confident that cleanup and disposal of the object has been done and you can then de-reference the object.

A side note, if this is structured how your code is actually laid out you will get an exception if something assigns null to your CurrentPerson property if Person is nullable.

Memory leaks because of event handlers

Use the Weak Event pattern (don't know if it's possible with SL). This will allow a dirtier programming model with calling your static method.

A better alternative whould be to properly subscribe / unsubscribe events (agreeing with tomasmcguinness), but this implies you can change the calling code and/or ask the consumers to better develop.

Another solution could to use async patterns. Maybe waiting for a Callback delegate in your method call, or even better, return a Task object.

Why this does not cause a memory leak when event is not unsubscribed

This is a case of a circular reference. The form has a reference to the object that listens to the event through the John field. In turn, John has a reference to the form when its UponWakingUp event was subscribed by the form's constructor.

Circular references can be a problem in certain automatic memory management schemes, particularly so in reference counting. But the .NET garbage collector doesn't have a problem with. As long as neither the form object nor the Person object have any additional references, the circular reference between the two cannot keep each other alive.

There are no additional references to either in your code. Which would normally cause both objects to be garbage collected. But the Form class is special, as long as a native Windows window for it exists, an internal reference stored in a handle-to-object table maintained by Winforms keeps the form object alive. Which keeps John alive.

So the normal way this is cleaned-up is that the user closes the window by clicking the X in the upper right corner. Which in turn causes the native window handle to be destroyed. Which removes the form reference from that internal table. The next garbage collection now sees nothing but the circular reference and collects them both.

Why and How to Avoid Event Handler Memory Leaks