Posts
48
Comments
146
Trackbacks
0
Beware Singletons that Raise Events

It’s pretty well documented that one of the most common ways in which a .NET application can “leak” memory stems failing to unsubscribe from events when the subscription is no longer needed. This answer to a Stack Overflow question on memory leaks in C# describes the issue very succinctly:

Event Handlers are a very common source of non-obvious memory leaks. If you subscribe to an event on object1 from object2, then do object2.Dispose() and pretend it doesn't exist (and drop out all references from your code), there is an implicit reference in object1's event that will prevent object2 from being garbage collected.

MyType object2 = new MyType();

// ...
object1.SomeEvent += object2.myEventHandler;
// ...

// Should call this
// object1.SomeEvent -= object2.myEventHandler;

object2.Dispose();
This is a common case of a leak - forgetting to easily unsubscribe from events. 
Of course, if object1 gets collected, object2 will get collected as well, but not until then.
Original question by user Joan Venge. Answer by user Reed Copsey.

Note: It’s probably debatable as to whether or not the event handler scenario illustrated above represents a true memory leak or not. Some would argue that the garbage collector takes care of nearly all memory leaks that might have otherwise occurred in unmanaged code and that the event handler issue isn’t really a memory leak in the strictest sense of the term. In my opinion, having event handlers hold references to objects and preventing them from being garbage collected is going to cause the memory used by the application to grow unnecessarily and that is, for lack of a better term, a “memory leak”.

How often do you make sure to “-=” an event handler that you subscribed to with the “+=” operator? I for one know that un-hooking an event handler is rarely something that I give a lot of thought to when writing an application. Have I been inadvertently causing memory leaks by failing to do so?

Probably not. The indomitable Jon Skeet points out why these types of memory issues are rarely cause for concern in an answer to another Stack Overflow question related to event handler memory leaks asked by user gillyb:

… 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.

I agree with Jon’s sentiment that this event handler “leak” is not typically a cause for concern and that you can usually get away without explicitly unsubscribing from events without causing memory issues at runtime. Recently, however, I came across a situation where an event subscription was causing a certain type of object (with a fairly large number of other objects in its graph) to remain in memory indefinitely causing pretty severe memory bloat in the application. In this case there was an object that raised events implemented as a singleton. The event raised by the singleton had many other instances of shorter-lived objects subscribe to it. When the shorter-lived objects fell out of scope the garbage collector was unable to clean them up because of the reference to the singleton’s event.

Example: Gym Membership System

Whenever I go to my gym to workout I have to check-in by scanning my membership ID at the main reception desk. The computer connected to the barcode scanner beeps, and my membership profile briefly comes up on the screen so that the staff can see my picture and verify that I’m the owner of the membership ID that was scanned. If I have to make a change to my account (like updating the expiration date on the credit card that they charge my membership fees to), they have to exit the “check-in” screen and pull up my account in another area of the application. I have no idea how this piece of software was built, but given what I know about how it’s typically used I can speculate about how it might be susceptible the type of memory leak that I’m talking about in this post.

To keep things simple for illustrative purposes, let’s say that this gym membership system consists of four total classes:

  1. A main menu class (form for launching the other forms)
  2. An account lookup class (form for finding and changing account details)
  3. A member check-in class (form that accepts input of membership ID from the scanner, looks up the membership profile and shows it on the screen)
  4. A scanner class (wrapper class for interfacing with the drivers for the laser barcode scanner that sits on the reception desk)

For the purposes of this example let’s say that the main menu, account lookup, and member check-in classes are all very simply implemented as Windows Forms.  The particulars of the implementation aren’t important, so let’s just call these classes the views of our application. Let’s also say that, for the purposes of this example, these views are only a very small subset of the views contained by the application and that users of the application are likely to have to switch between these different views many times throughout the course of the day. In any given day the application might have to display several dozen different views to the user so the application creates views on-the-fly as needed and disposes of them as soon as they are no longer needed. Put another way, only one view should need to be actively in-use at one time, and therefore any view that is no longer needed should immediately fall out of scope and be a candidate for garbage collection. The one exception to this design is the main menu, as users will continually return to the menu in order to bring up the next view that they need so the main menu view will serve as the main entry point for the application and have a single instance that will remain in memory for as long as the application is running.

The scanner class, on the other hand, is implemented as a singleton. The application only needs to support taking input from one scanner that is connected to the PC at the member services desk via USB. Initializing the scanner drivers and hardware can take a few seconds so in order to save time the single instance of the scanner gets initialized at application startup and lives for the duration of the app. The scanner class and supporting types might look something like this:

   1:      public class ScanReceivedArgs
   2:      {
   3:          public string ScanString { get; set; }
   4:      }
   5:      public interface IScanner
   6:      {
   7:          event EventHandler<ScanReceivedArgs> ScanReceived;
   8:      }
   9:   
  10:      public class DesktopScanner : IScanner
  11:      {
  12:          private static readonly DesktopScanner instance = new DesktopScanner();
  13:          private DesktopScanner(){}
  14:   
  15:          public static DesktopScanner Instance {get { return instance; }}
  16:          public event EventHandler<ScanReceivedArgs> ScanReceived;
  17:      }

Obviously this code doesn’t really do anything and is missing all of the “guts” that would make it actually be able to talk to a piece of hardware, but we have enough here for the purposes of this example. There will be a single instance of the DesktopScanner class as enforced by the private constructor with a public “Instance” property to return the sole instance that the class spins up of itself. The IScanner interface that it implements exposes an event that will be fired when the scanner recognizes that a barcode has been placed in front of it. The arguments passed to the event contain the string representing the value that was scanned.

Now let’s see how the member check-in form might be implemented to make use of the scanner:

   1:      public partial class MemberCheckinForm : Form
   2:      {
   3:          public MemberCheckinForm()
   4:          {
   5:              InitializeComponent();
   6:          }
   7:   
   8:          private void MemberCheckinForm_Load(object sender, EventArgs e)
   9:          {
  10:              DesktopScanner.Instance.ScanReceived += ScanReceived;
  11:          }
  12:   
  13:          private void ScanReceived(object sender, ScanReceivedArgs args)
  14:          {
  15:              //look up member details to display on screen...
  16:          }
  17:      }

When the form loads, we subscribe to the scanner’s ScanReceived event and point it toward a method that will take the arguments and look up the member details associated with the barcode that was scanned. As stated previously, there are a large number of other views used by this application, including an account lookup view. I won’t bother showing any implementation details for those views but suffice it to say that they do not use the scanner’s ScanReceived event.

After running the application for little while and switching between the various views it becomes evident that the memory used by the application continues to grow steadily until the application is shut down. The gym is open for very long hours during the day and the staff can’t be bothered to constantly shut down and re-start the application then things start to slow down. There appears to be a memory leak of some kind that needs to be fixed.

By using the excellent ANTS Memory Profiler tool from Red Gate Software we can attach to the running application and take a snapshot to see what objects are being held in memory. Here’s what the profiler tells us:

image

Here we can see all of the types being used by our application along with their size in byes and how many live instances there are of each. As expected, we see that the DesktopScanner and MainMenu classes have a single live instance. What’s unexpected, however, is that there are 11 instances of the MemberCheckinForm. If we are properly creating and disposing all of our views as they are needed, we shouldn’t have these instances sticking around in memory. It’s also important to note here that none of the other view classes are showing up in this snapshot.

We can drill down into the details a bit further to see what is keeping these instances of the MemberCheckinForm from being garbage collected:

image

Here in the Instance List view we can see that each instance of the MemberCheckinForm is not a GC root object (the “root” object that, for the purposes of garbage collection, is keeping the class from being cleaned up) and is 5 references away from the GC root object that is keeping it from being garbage collected. By clicking on the Instance Retention Graph we can see a graphical representation of the object that is serving as the GC root for these instances:

image

Here we can see plain as day that the MemberCheckinForm is being held by our DesktopScanner’s ScanReceived event.

So now we know what the problem is, how can we go about fixing it? There are a few different approaches we could take:

  1. Implement the MemberCheckinForm as a singleton: This would probably work in this very simple example, but it goes against the general design of the application that’s trying to create and dispose views on an as-needed basis. In my opinion this is not the right way to go here.
  2. Use a “weak event handler” pattern: There have been some attempts to create “weak event handlers” in .NET. The idea here is that you create event handlers that will not keep an object that has subscribed to an event from being garbage collected. This is an interesting idea and is something that I might explore further in another post. If you’re interested, a Google search for “weak events in .NET” should yield plenty of good information and example implementations of varying quality and completeness.
  3. Update the MemberCheckinForm to unsubscribe from the event: The simplest approach to dealing with this issue is to simply update the MemberCheckinForm so that it will unsubscribe from the event when it gets disposed. This way, as long as each instance of MemberCheckinForm is getting disposed properly (which it should be anyway) it will also break the reference it has to the long-lived scanner class.

In order to have the MemberCheckinForm unsubscribe from the scanner event we can just move the Dispose method out of the partial “designer.cs” file for the form and modify it to unsubscribe from the event. The full body of the modified class looks like this:

   1:     public partial class MemberCheckinForm : Form
   2:      {
   3:          public MemberCheckinForm()
   4:          {
   5:              InitializeComponent();
   6:          }
   7:   
   8:          private void MemberCheckinForm_Load(object sender, EventArgs e)
   9:          {
  10:              DesktopScanner.Instance.ScanReceived += ScanReceived;
  11:          }
  12:   
  13:          private void ScanReceived(object sender, ScanReceivedArgs args)
  14:          {
  15:              //look up member details to display on screen...
  16:          }
  17:   
  18:          /// <summary>
  19:          /// Clean up any resources being used.
  20:          /// </summary>
  21:          /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
  22:          protected override void Dispose(bool disposing)
  23:          {
  24:              DesktopScanner.Instance.ScanReceived -= ScanReceived;
  25:              if (disposing && (components != null))
  26:              {
  27:                  components.Dispose();
  28:              }
  29:              base.Dispose(disposing);
  30:          }
  31:      }

Adding the “DesktopScanner.Instance.ScanReceived –= ScanReceived” makes all the difference and ensures that the MemberCheckinForm can be garbage collected properly.

The moral of the story here is that a singleton that raises an event can work well, but care should be taken to ensure that subscribers to that event are cleanly unsubscribing when they fall out of scope.

posted on Sunday, January 27, 2013 9:32 PM Print
Comments
Gravatar
# re: Beware Singletons that Raise Events
Misty Fowler
1/28/2013 5:37 PM
Are you aware that this page is requiring a horizontal scrollbar? I'm in Chrome, with a resolution (on my laptop) of 1280 x 800.
Gravatar
# re: Beware Singletons that Raise Events
Jesse
1/29/2013 8:41 AM
Thanks for pointing that out, Misty. I think I've corrected that now.

Post Comment

Title *
Name *
Email
Comment *  
 
Meta
Tag Cloud