WeakEventManager<TEventSource, TEventArgs> and PropertyChangedEventManager causes memory leak

Question

I've an application where I'm not able to remove event handlers because I don't know when the last reference will be freed.

My application contains a PropertyChanged event source that is put into a container class that also implements INotifyPropertyChanged. This hierarchy contains more than 6 levels. Each instance of a level could be placed into multiple other instances. That's the reason why I couldn't determine when to free those instances.

The instances on the lowest level will live for the whole application runtime. This causes that all other instances will not be freed and I got a memory leak.

To avoid this event driven memory leak I tried to use WeakEventManager(TEventSource, TEventArgs). This class is only available in .Net 4.5 and because of compatibility to existing hardware I’ve to use .Net 4.0.

In .Net 4.0 is a PropertyChangedEventManager available that should do the same for INotifyPropertyChanged.

My classes are freed correctly.

But there is still a memory leak.

I simplified my application to the following code that produces a memory leak:

// This code will force the memory leak
while (true)
{
    var eventSource = new StateChangedEventSource();
    var eventReceiver = new StateChangedEventReceiver();

    PropertyChangedEventManager.AddListener(eventSource, eventReceiver, string.Empty);
}

public class EventSource : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;
}

public class  EventReceiver : IWeakEventListener
{
    public bool ReceiveWeakEvent(Type managerType, object sender, EventArgs e)
    {
        return true;
    }
}

Yes I know there is no RemoveListener call. I couldn’t determine when an instance is never used and could be freed. If I knew that I could use normal event registration and deregistration. In that case I don’t have to use the PropertyChangedEventManager.

What is the problem of my sample code? Why does it produce a memory leak?

Edit 2014/02/17:

I tried the WeakEventManager(TEventSource, TEventArgs) and .Net 4.5 and the problem still exists.

var eventSource = new EventSource();

var i = 0;
while (true)
{
    var eventReceiver = new EventReceiver();

    // --> Use only one of the following three lines. Each of them will produce a memory leak.
    WeakEventManager<EventSource, PropertyChangedEventArgs>.AddHandler(eventSource, "PropertyChanged", eventReceiver.OnEvent);
    PropertyChangedEventManager.AddListener(eventSource, eventReceiver, string.Empty);
    WeakEventManager<EventSource, EventArgs>.AddHandler(eventSource, "SomeOtherEvent", eventReceiver.OnSomeOtherEvent);
    // <--

    ++i;
    if (i == 1 << 18)
    {
        Thread.Sleep(10);
        GC.Collect(2);
        Thread.Sleep(10);
        i = 0;
    }
}


public class EventSource : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    public event EventHandler<EventArgs> SomeOtherEvent;
}

public class EventReceiver : IWeakEventListener
{
    public void OnSomeOtherEvent(object sender, EventArgs args)
    {
    }

    public void OnEvent(object sender, PropertyChangedEventArgs args)
    {
    }

    public bool ReceiveWeakEvent(Type managerType, object sender, EventArgs e)
    {
        return true;
    }
}

This code compiled using .Net 4.5 do also run out of memory. I got the hint using the Thread.Sleep construct here.

Answer 1

I don't think the isue with WeakEventManager<,> is specific to non-WPF, as i can reproduce a memory leak in a WPF application as well.

The problem lies with the management of the event table. For each subscription, the WeakEventManager creates an entry in a table. This entry and table are (by necessity) strong-referenced.

The problem is, that by default, the WeakEventManager does not clean up the records. You have to call RemoveHandler. But beware. It's not thread safe. If you call it from another thread it may fail (doesn't throw an exception, you'll just experience that there's still a memory leak). When called from a finalizer, it doesn't work reliably either.

I also investigated into the source code and found that while it contains logic to do cleanup on AddHandler and when an event is received, it is disabled by default (see WeakEventManager.cs => WeakEventTable.CurrentWeakEventTable.IsCleanupEnabled). Also, you can't access the Cleanup method since the methods and properties necessary to do so are private or internal. So you can't even create subclass to access these methods / modify the behavior.

WeakEventManager<,> is broken

So basically (as far as i can understand) WeakEventManager<,> is broken by design (it keeps a StrongReference to the subscriber table-entry). Instead of fixing a MemoryLeak it will only reduce the MemoryLeak (the event source and listener can be garbage collected, but the entry for the event subscription is not => new memory leak). Of course the memory leak introduced by WeakEventManager<,> is small.

Answer 2

Based on the information at msdn and codeproject I realized that the WeakEventManager(TEventSource, TEventArgs) class will only work in WPF applications. I'm using WinForms what's the reason why it seems not to work.

I decided to create my own WeakEventManager that works without using the build in WeakEventManager provided by the .Net framework.

The implementation of my WeakEventManager uses a background thread to clean up all instances. Maybe there's a better solution but this solution will work correctly in my application.

public static class ThreadedWeakEventManager
{
    private static readonly TimeSpan CleanupInterval = TimeSpan.FromSeconds(1.0);

    private static readonly List<IInternalWeakEventManager> EventManagers = new List<IInternalWeakEventManager>();

    private static volatile bool _performCleanup = true;

    static ThreadedWeakEventManager()
    {
        new Thread(Cleanup) { IsBackground = true, Priority = ThreadPriority.Lowest }.Start();
    }

    public static void AddHandler<TEventArgs>(object eventSource, string eventName, EventHandler<TEventArgs> eventHandler)
        where TEventArgs : EventArgs
    {
        var weakEventManager = new InternalWeakEventManager<TEventArgs>(eventSource, eventName, eventHandler);

        lock (EventManagers)
        {
            EventManagers.Add(weakEventManager);
        }
    }

    public static void AddPropertyChangedHandler(INotifyPropertyChanged eventSource, EventHandler<PropertyChangedEventArgs> eventHandler)
    {
        AddHandler(eventSource, "PropertyChanged", eventHandler);
    }

    public static void AddCollectionChangedEventHandler(INotifyCollectionChanged eventSource, EventHandler<NotifyCollectionChangedEventArgs> eventHandler)
    {
        AddHandler(eventSource, "CollectionChanged", eventHandler);
    }

    public static void RemoveHandler<TEventArgs>(object eventSource, string eventName, EventHandler<TEventArgs> eventHandler)
        where TEventArgs : EventArgs
    {
        if (eventSource == null || string.IsNullOrWhiteSpace(eventName) || eventHandler == null)
        {
            return;
        }

        lock (EventManagers)
        {
            EventManagers.RemoveAll(item => object.ReferenceEquals(item.EventData.EventSource, eventSource) && item.EventName.Equals(eventName) && eventHandler.Method.Equals(item.EventData.EventHandlerMethodInfo));
        }
    }

    public static void RemovePropertyChangedHandler(INotifyPropertyChanged eventSource, EventHandler<PropertyChangedEventArgs> eventHandler)
    {
        RemoveHandler(eventSource, "PropertyChanged", eventHandler);
    }

    public static void RemoveCollectionChangedEventHandler(INotifyCollectionChanged eventSource, EventHandler<NotifyCollectionChangedEventArgs> eventHandler)
    {
        RemoveHandler(eventSource, "CollectionChanged", eventHandler);
    }

    public static void CancelCleanup()
    {
        _performCleanup = false;
    }

    private static void Cleanup()
    {
        while (_performCleanup)
        {
            Thread.Sleep(CleanupInterval);

            lock (EventManagers)
            {
                for (var i = EventManagers.Count - 1; i >= 0; --i)
                {
                    var item = EventManagers[i];
                    if (item.EventData.IsGarbageCollected)
                    {
                        item.UnwireEvent();
                        EventManagers.RemoveAt(i);
                    }
                }
            }
        }
    }

    private interface IInternalWeakEventManager
    {
        string EventName { get; }
        IWeakEventData EventData { get; }>
        void UnwireEvent();
        void OnEvent(object sender, EventArgs args);
    }

    private class InternalWeakEventManager<TEventArgs> : IInternalWeakEventManager
        where TEventArgs : EventArgs
    {
        private static readonly MethodInfo OnEventMethodInfo = typeof(InternalWeakEventManager<TEventArgs>).GetMethod("OnEvent");
        private EventInfo _eventInfo;
        private Delegate _onEvent;

        public InternalWeakEventManager(object eventSource, string eventName, EventHandler<TEventArgs> eventHandler)
        {
            this.EventData = new WeakEventData<TEventArgs>(eventSource, eventHandler);

            this.WireEvent(eventSource, eventName);
        }

        public string EventName
        {
            get { return this._eventInfo.Name; }
        }

        public IWeakEventData EventData { get; private set; }

        public void UnwireEvent()
        {
            var eventSource = this.EventData.EventSource;
            if (eventSource == null)
            {
                return;
            }

            this._eventInfo.RemoveEventHandler(eventSource, this._onEvent);
        }

        public void OnEvent(object sender, EventArgs args)
        {
            this.EventData.ForwardEvent(sender, args);
        }

        private void WireEvent(object eventSource, string eventName)
        {
            this._eventInfo = eventSource.GetType().GetEvents().FirstOrDefault(item => item.Name == eventName);
            if (this._eventInfo == null)
            {
                throw new InvalidOperationException(string.Format("The event source type {0} doesn't contain an event named {1}.", eventSource.GetType().FullName, eventName));
            }

            this._onEvent = Delegate.CreateDelegate(this._eventInfo.EventHandlerType, this, OnEventMethodInfo);
            this._eventInfo.AddEventHandler(eventSource, this._onEvent);
        }
    }

    private interface IWeakEventData
    {
        bool IsGarbageCollected { get; }
        object EventSource { get; }>
        MethodInfo EventHandlerMethodInfo { get; }
        void ForwardEvent(object sender, EventArgs args);
    }

    private class WeakEventData<TEventArgs> : IWeakEventData
        where TEventArgs : EventArgs
    {
        private readonly WeakReference _eventSource;
        private readonly WeakReference _eventTargetInstance;

        public WeakEventData(object eventSource, EventHandler<TEventArgs> eventHandler)
        {
            this._eventSource = new WeakReference(eventSource);
            this._eventTargetInstance = new WeakReference(eventHandler.Target);
            this.EventHandlerMethodInfo = eventHandler.Method;
        }

        public object EventSource
        {
            get { return this._eventSource.Target; }
        }

        public MethodInfo EventHandlerMethodInfo { get; private set; }

        public bool IsGarbageCollected
        {
            get
            {
                return !this._eventSource.IsAlive || !this._eventTargetInstance.IsAlive;
            }
        }

        public void ForwardEvent(object sender, EventArgs args)
        {
            var target = this._eventTargetInstance.Target;
            if (target != null)
            {
                this.EventHandlerMethodInfo.Invoke(target, new[] { sender, args });
            }
        }
    }
}