Recently I have been developing custom objects that expose numerous events and there were several issues I needed to address or consider:
- What are the 'Best Practices' for handling events?
- What happens to memory when there are a large number of events?
- If the object closes, what happens to any event delegates still registered if the listener did not unregister the events? Will the GC collect the disposed object? In other words, how do I properly dispose of all those events if they are still registered?
- Are my events thread-safe?
Let's try to establish a 'Best Practice' by looking at typical event declarations:
Code Snippet
public class NonDisposableClass {
public NonDisposableClass() {
}
public event EventHandler<EventArgs> SomeEvent;
protected virtual void OnSomeEvent(EventArgs ea) {
if (SomeEvent != null)
SomeEvent(this, ea);
}
}
Here is a sample of a class (listener) registering with the event:
Code Snippet
public class NonDisposableEventListener {
public NonDisposableEventListener() {
RegisterEvent();
}
// A reference to the non disposable class exposing an event
private NonDisposableClass m_nonDisposableClass = new NonDisposableClass();
void RegisterEvent() {
m_nonDisposableClass.SomeEvent += new EventHandler<EventArgs>(m_nonDisposableClass_SomeEvent);
}
void m_nonDisposableClass_SomeEvent(object sender, EventArgs e) {
// Do something when event occurs here...
}
}
These samples work with each other, but are they the 'Best Practice'?
First, we need to understand that the C# keyword event is special and allows this line of code in the NonDisposableEventListener class to work:
Code Snippet
m_nonDisposableClass.SomeEvent += new EventHandler<EventArgs>(m_nonDisposableClass_SomeEvent);
Under the hood (or in the resulting IL), the C# compiler is creating a delegate with Add/Remove handlers that allow the += or -= operators to work. More than that and importantly... this style of event declaration has some serious potential for problems. Let's begin with a look at memory usage; here is a quote from this link : .NET Matters
"This hints at one of the primary reasons for writing your own add and remove accessors: to provide your own underlying data store. One reason you might want to do this [use EventHandlersList] is if you have lots of exposed events on your class, but in such a way that only a few are typically in use on an instance at any point in time. In such a scenario, there can be significant memory overhead associated with maintaining a delegate field for each event. Take the Windows® Forms Control class as an example.
In the Microsoft .NET Framework 2.0, Control exposes 69 public events. If each of these events had an underlying delegate field, on a 32-bit system these events would add an overhead of 276 bytes per instance. Instead, Control (or, more specifically, its base class System.ComponentModel.Component) maintains a list of key/value pairs, where the value is a delegate. Every event on Control then has custom add and remove accessors that store the registered delegates for all events into this list, an instance of the System.ComponentModel.EventHandlersList class. "
Huh? What is the issue?
MEMORY: Well, what I believe the author is stating is that events declared in such a manner as the typical example I provided will cause the C# compiler to reserve memory for the event's delegate regardless of whether a listener has actually registered with the event. With numerous events, the available memory is reduced and can have a significant impact on performance when resources are tight. *I will address a solution after examining other issues.
DISPOSING: Next we should look at the example again and consider what happens when they are disposed (out of scope): in the example, there was no code added to specifically manage resources on disposal, but it was not necessary because the class broadcasting the events is created within the class receiving the events. There is an inherent mutual dependency and they coexist and are created and disposed together...
However, what if there is some form of Dependency Injection where the class broadcasting the events is instantiated outside the class and its reference is added via the constructor? Would problems possibly arise? Let's explore this scenario:
If the listener class registers with the event, under the hood, there is a link or reference to each other maintained between each class once the listener has registered with the event. If the listener class goes out of scope (is no longer in use), the GC (Garbage Collector) may not identify the class for garbage collection because of this underlying reference and may keep it alive for as long as the NonDisposableClass is still alive. Conversely, if the class responsible for announcing the event (the NonDisposableClass) goes out of scope it may not be garbage collected because of the reference to the listener. In essence, there is potential for mutual dependency that results in both classes remaining in memory until both are disposed or removed from scope. When there are multiple listeners and multiple events, this can become very significant and the previous example would expose potential 'memory leaks' - unintended storage or references of objects that hang around in memory significantly longer than desired.
Let's expand my example to include disposal when a form of dependency injection exists (note the constructor):
Code Snippet
public class DisposableEventListener : IDisposable {
public DisposableEventListener(NonDisposableClass nonDisposableClass) {
if (nonDisposableClass == null)
throw new ArgumentNullException("nonDisposableClass");
m_nonDisposableClass = nonDisposableClass;
RegisterEvent();
}
// A reference to the non disposable class exposing an event
private NonDisposableClass m_nonDisposableClass;
protected virtual void RegisterEvent() {
m_nonDisposableClass.SomeEvent += new EventHandler<EventArgs>(m_nonDisposableClass_SomeEvent);
}
void m_nonDisposableClass_SomeEvent(object sender, EventArgs e) {
// Do something when event occurs here...
}
public void Dispose() {
Dispose(true);
GC.SuppressFinalize(this);
}
private bool IsDisposed = false;
protected void Dispose(bool Disposing) {
if (!IsDisposed) {
if (Disposing) {
//Clean Up managed resources
// Ensure we clean-up references to event
if (m_nonDisposableClass != null)
m_nonDisposableClass.SomeEvent -= new EventHandler<EventArgs>(m_nonDisposableClass_SomeEvent);
}
//Clean up unmanaged resources
}
IsDisposed = true;
}
~DisposableEventListener() {
Dispose(false);
}
}
Ah, we are getting somewhere. Now, the listener takes on the responsibility of ensuring that it unregisters it own event(s) (which is the base for the underlying mutual reference between the listener and the class announcing the event and can prevent proper garbage collection). PROPER DISPOSAL IS CRUCIAL WHEN REFERENCES EXIST OUTSIDE THE CLASS.
Further, I did not find an example where the class broadcasting events also takes on the responsibility of removing any registered listeners when it is disposed. Should it? Well, if listeners are outside the control of the developer creating the class broadcasting events, I would say yes. In fact, I believe it is a 'Best Practice' even when the developer is in control of the listeners. Consider my dependency injection sample where some outside class that is inaccessible from listener is managing the life-cycle of the class broadcasting events. What if this manager class is disposed? All the underlying references for events could keep a big chain alive in memory as all of the listeners are unaware of the disposal and are still registered. Here is a sample with the class broadcasting events handling the disposal of its registered listeners:
Code Snippet
public class DisposableEventClass : IDisposable {
public DisposableEventClass() {
}
public event EventHandler<EventArgs> SomeEvent;
protected virtual void OnSomeEvent(EventArgs ea) {
if (SomeEvent != null)
SomeEvent(this, ea);
}
public void Dispose() {
Dispose(true);
GC.SuppressFinalize(this);
}
private bool IsDisposed = false;
protected void Dispose(bool Disposing) {
if (!IsDisposed) {
if (Disposing) {
//Clean Up managed resources
// Ensure we clean-up references to event
SomeEvent = null;
}
//Clean up unmanaged resources
}
IsDisposed = true;
}
~DisposableEventClass() {
Dispose(false);
}
}
Whew! Now we ensure that we a class responsible for broadcasting events ensures that all registered event listener references are disposed.
*** I will not provide another example because I think the reader can understand what is needed, but, a 'Best Practice' should also include, at a minimum) a 'Closed' event for classes that broadcast events. This way listeners are informed that the broadcasting class is no longer available to broadcast events and the listeners will be able to react to such a scenario appropriately
THREAD-SAFETY: If we deploy the latter code examples, would the events be thread-safe? The short answer is NO. A longer answer involves the developer understanding the specific EventArgs passed via the events and the resulting actions taken based on the events. If there are any (even one) shared member altered due to the events, a potential for problems related to thread-safety exists. If, for example, there is a static counter shared across instances,one thread could alter the counter just after another new thread accesses the counter value and weird, unwanted, incorrect behavior can occur.
I found a nice reference to a code snippet for events on this link:
Here is the snippet:
Code Snippet
public class ThreadSafeEventHandler {
public ThreadSafeEventHandler() {
}
private readonly object TextChangedEventLock = new object();
private EventHandler<EventArgs> TextChangedEvent;
/// <summary>
/// Event raised after the <see cref="Text" /> property value has changed.
/// </summary>
public event EventHandler<EventArgs> TextChanged {
add {
lock (TextChangedEventLock) {
TextChangedEvent += value;
}
}
remove {
lock (TextChangedEventLock) {
TextChangedEvent -= value;
}
}
}
/// <summary>
/// Raises the <see cref="TextChanged" /> event.
/// </summary>
/// <param name="e"><see cref="EventArgs" /> object that provides the arguments for the event.</param>
protected virtual void OnTextChanged(EventArgs e) {
EventHandler<EventArgs> handler = null;
lock (TextChangedEventLock) {
handler = TextChangedEvent;
if (handler == null)
return;
}
handler(this, e);
}
}
Notice the changes where we manually handle the add/remove methods for the event and our new implementation wraps them in a lock. Also notice that calls to OnTextChanged (raising the event) is also uses a lock. This combination ensures that our events are thread-safe.
MEMORY-REVISITED: I previously delayed addressing the memory issue where numerous events can result in less-than-desirable memory usage. What can we do? Well, let's look at Microsoft's solution that they implemented on the Control class. Within the framework is a handy class for managing multiple events, the System.ComponentModel.EventHandlerList. If you create a Windows form and type 'this' (dot), intellisense will reveal a property named 'Events'. Events is simply a reference to an instance of the EventHandlerList. Using the list to stores events frees up memory for the events that are not registered. It also has a bonus feature where its much easier to dispose of all events in one call by setting Events = null in the dispose method rather than each respective event. If you are using a class that inherits from control, you should use the Events property to add/remove events, rather than use private fields.
Here is a sample using the Control class:
Code Snippet
public class ControlEventSample : Control {
public ControlEventSample() {
}
private readonly object TextChangedEvent = new object();
/// <summary>
/// Event raised after the <see cref="Text" /> property value has changed.
/// </summary>
[Category("Property Changed")]
[Description("Event raised after the Text property value has changed.")]
public event EventHandler<EventArgs> TextChanged {
add {
lock (TextChangedEvent) {
Events.AddHandler(TextChangedEvent, value);
}
}
remove {
lock (TextChangedEvent) {
Events.RemoveHandler(TextChangedEvent, value);
}
}
}
/// <summary>
/// Raises the <see cref="TextChanged" /> event.
/// </summary>
/// <param name="e"><see cref="EventArgs" /> object that provides the arguments for the event.</param>
protected virtual void OnTextChanged(EventArgs e) {
EventHandler<EventArgs> handler = null;
lock (TextChangedEvent) {
handler = (EventHandler<EventArgs>)Events[TextChangedEvent];
}
if (handler != null)
handler(this, e);
}
}
Okay, so what if you don't inherit from a built-in class where the Events property is already implemented? Can you still use this class? The answer: Yes!
Simply add this property to your class:
Code Snippet
public EventHandlerList Events {
get {
return m_events;
}
}
private EventHandlerList m_events = new EventHandlerList();
STATIC VERSUS INSTANCE EVENTS: Events like other members of a class can be declared as static. I caution developers to be careful when using the System.ComponentModel.EventHandlerList and warn not use the EventHandlerList to manage static events. Manage them using private static fields. The reason I add such a caution is the handy way of disposing all registered events by setting Events to null on disposal of an instance. If static events are referenced in the EventHandlerList and multiple instances are open, then any instance closing would unregister all other open instances and this is clearly unwanted behavior.
CONCLUSION: Events are a powerful feature of the .NET Framework, but perils exists if they are not understood and implemented properly. I shared what I believe addresses potential pitfalls regarding memory optimization, thread-safety, proper disposal, and static versus instance events. I hope my post helps others and am open to any comments or suggestions others may provide.
DISCLAIMER: THE CODE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE CODE OR THE USE OR OTHER DEALINGS WITH THE CODE.