locked
inline eventhandler registration - potential for memory leaks? RRS feed

  • Question

  • Hi

    Suppose I have a class exposing an event

    public event EventHandler<InstantMessageSentEventArgs> InstantMessageSent;

    And I'm registering for event notification using

    connector.InstantMessageSent += connector_InstantMessageSent;
    
    void connector_InstantMessageSent(object sender, InstantMessageSentEventArgs e)
            {
                // event processing
            }

    And then I'd like to tear the event handler down so as to not create memory leaks. Will

    connector.InstantMessageSent -= connector_InstantMessageSent;

    do the trick, or do I have to have to

    private EventHandler<InstantMessageSentEventArgs> instantMessageSentEventHandler = new new EventHandler<InstantMessageSentEventArgs>(connector_InstantMessageSent);
    

    and then register and unregister using

    connector.InstantMessageSent += instantMessageSentEventHandler;
    ...
    connector.InstantMessageSent -= instantMessageSentEventHandler;

    I've been using the latter approach, but with Visual Studio 2012, it automatically generates the first syntax (which I actually prefer) but having had plenty of memory leaks due to not unregistering event handlers, I'm a bit worried.

    Monday, February 4, 2013 10:40 AM

Answers

  • Hi,

    as long as you add a simple method as event handler, you can simply add with += and remove with -=. Just test it yourself with a small test application like this:

        class Program
        {
            public event EventHandler<EventArgs> TestEvent;
    
            static void Main(string[] args)
            {
                var prog = new Program();
                prog.Test();
            }
    
            public void Test()
            {
                if (TestEvent != null)
                    Console.WriteLine("Current Length: {0}", TestEvent.GetInvocationList().Length);
    
                TestEvent += TestEventHandler;
                Console.WriteLine("Length after insert: {0}", TestEvent.GetInvocationList().Length);
    
                TestEvent -= TestEventHandler;
                if (TestEvent != null)
                    Console.WriteLine("Length after removal: {0}", TestEvent.GetInvocationList().Length);
                else
                    Console.WriteLine("No entries!");
            }
    
            public void TestEventHandler(object sender, EventArgs args)
            {
                // Nothing to do
            }
        }
    

    You will see that the removal is done correctly.

    But as soon as you create an anonymous method, you must store the method created somehow. An example could be something like this:

    someObject.AnEvent += ((sender, args) => Invoke(new EventHandler<TheEventArgs>(MyFunction), sender, args));

    (This is the syntax we are using in multithreaded applications when the GUI subscibes to events of business objects.)

    The core problem is, that an annonymous method is created and when we try the same line with -= instead of the +=, another method is created. The second methof to call is not the first one so the first one cannot be removed.

    With kind regards,

    Konrad

    • Proposed as answer by Reed KimbleMVP Monday, February 4, 2013 4:19 PM
    • Marked as answer by Mike Feng Tuesday, February 12, 2013 2:46 AM
    Monday, February 4, 2013 11:47 AM
  • The first syntax is just to make it easier to set event handler. The functionality is same so it really does not matter which one you use.

    /// this is same just simplified syntax
    button.Click += this.button_Click;
    button.Click += new EventHandler(button_Click);
    
    
    You can also declare variable to hold the event handler delegate, but that's not necessary. The previous versions of Visual Studio have not done that either as desinger generated event handlers are usually been created by latter syntax in my code snippet.

    • Proposed as answer by Reed KimbleMVP Monday, February 4, 2013 4:19 PM
    • Marked as answer by Mike Feng Tuesday, February 12, 2013 2:46 AM
    Monday, February 4, 2013 11:22 AM

All replies

  • The first syntax is just to make it easier to set event handler. The functionality is same so it really does not matter which one you use.

    /// this is same just simplified syntax
    button.Click += this.button_Click;
    button.Click += new EventHandler(button_Click);
    
    
    You can also declare variable to hold the event handler delegate, but that's not necessary. The previous versions of Visual Studio have not done that either as desinger generated event handlers are usually been created by latter syntax in my code snippet.

    • Proposed as answer by Reed KimbleMVP Monday, February 4, 2013 4:19 PM
    • Marked as answer by Mike Feng Tuesday, February 12, 2013 2:46 AM
    Monday, February 4, 2013 11:22 AM
  • Hi,

    as long as you add a simple method as event handler, you can simply add with += and remove with -=. Just test it yourself with a small test application like this:

        class Program
        {
            public event EventHandler<EventArgs> TestEvent;
    
            static void Main(string[] args)
            {
                var prog = new Program();
                prog.Test();
            }
    
            public void Test()
            {
                if (TestEvent != null)
                    Console.WriteLine("Current Length: {0}", TestEvent.GetInvocationList().Length);
    
                TestEvent += TestEventHandler;
                Console.WriteLine("Length after insert: {0}", TestEvent.GetInvocationList().Length);
    
                TestEvent -= TestEventHandler;
                if (TestEvent != null)
                    Console.WriteLine("Length after removal: {0}", TestEvent.GetInvocationList().Length);
                else
                    Console.WriteLine("No entries!");
            }
    
            public void TestEventHandler(object sender, EventArgs args)
            {
                // Nothing to do
            }
        }
    

    You will see that the removal is done correctly.

    But as soon as you create an anonymous method, you must store the method created somehow. An example could be something like this:

    someObject.AnEvent += ((sender, args) => Invoke(new EventHandler<TheEventArgs>(MyFunction), sender, args));

    (This is the syntax we are using in multithreaded applications when the GUI subscibes to events of business objects.)

    The core problem is, that an annonymous method is created and when we try the same line with -= instead of the +=, another method is created. The second methof to call is not the first one so the first one cannot be removed.

    With kind regards,

    Konrad

    • Proposed as answer by Reed KimbleMVP Monday, February 4, 2013 4:19 PM
    • Marked as answer by Mike Feng Tuesday, February 12, 2013 2:46 AM
    Monday, February 4, 2013 11:47 AM
  • You talk about memory leaks because of this. 

    I've seen a lot but this is new to me. 

    Can you therefore explain where you recognize the memory leaks and maybe create a little piece of elaborating code?


    Success
    Cor

    Monday, February 4, 2013 1:58 PM
  • Hi Cor,

    I think he simply means that objects, that are still subscribed to an event, will not be collected by the GC (which is correct because the event has a reference to the object).

    (So one pattern I saw in the past was the implementation of IDisposable on classes which could subscribe to other objects events just to make sure that the subscription is removed.)

    With kind regards,

    Konrad

    Monday, February 4, 2013 2:32 PM
  • Hello Konrad,

    Thanks, but is that a leak, more something like this.


    This I call a leak :-)


    Success
    Cor

    Monday, February 4, 2013 3:11 PM
  • Cor, it could quickly turn into the second picture in the right scenario... especially a web application.

    There was recent an article in MSDN magazine that focused on this issue:

    http://msdn.microsoft.com/en-us/magazine/jj721593.aspx


    Reed Kimble - "When you do things right, people won't be sure you've done anything at all"

    Monday, February 4, 2013 4:09 PM
  • Reed,

    You must've seen how much I dislike weak reference situation (softly expressed). Many write webapplications with C# or VB as a kind of tool to create Javascript, JSON or MVC Basic and C#.

    That is why I make such a big difference between Web with real (complete) code behind (complete classes and no weak references) and that kind of webapplications, in my idea we see that back in the article you show. 

    But this is of course not a VB or C# forum, maybe I focus me to much on that.


    Success
    Cor

    Monday, February 4, 2013 4:37 PM