7

This question is a follow up to the C# Events and Thread Safety question (I am not the author of that one) and the related blog post by Eric Lippert Events and Races. There are other similar questions on SO, but none of them actually consider this case, general consensus is that so long as you unsubscribe you are safe, but I don't believe that is true all the time.

According to the discussion in both the SO question and the blog, the pattern that should be used is something like:

var ev = NotifyPropertyChanged;
if (ev != null)
    ev(this, new PropertyChangedEventArgs("Foo"));

But what if the following situation occurs:
1) I subscribe a listener:

mytype.NotifyPropertyChanged += Handler; // Handler is instance method in SomeObject class

2) I (or the run-time, due to scoping) dispose SomeObject, which contains the listener and unsubscribe the listener, at about the same time the property notification happens.

3) While unlikely due to very short time period this can happen in, it is theoretically possible that because ev preserves the old subscriber that is no longer in existence, it will call a function in an object that no longer exist.

According to the Eric Lippert, "event handlers are required to be robust in the face of being called even after the event has been unsubscribed". But if the handler is unsubscribed and disposed, it can no longer take care of the call. What is the correct way to handle this situation?

Wrap the code from (1) in try-catch? What exception should be caught? ObjectDisposedException seems likely, but not the only one that can happen, I think.

Community
  • 1
  • 1
Alex K
  • 10,835
  • 8
  • 29
  • 34
  • I kind of wonder if one could adopt a pattern that allows one to do this in event handlers: `if(Disposed) { /*get reference to caller's event and unsubscribe this object*/` since this scenario is a common source of memory leaks where someone has null'd out references to an object but forgot to unsubscribe it's handlers. – AaronLS Nov 30 '11 at 22:19
  • 1
    The question is confusing. By "disposed" do you mean that someone called Dispose() on the object, or that it was collected by the garbage collector, or that it was finalized by the garbage collector? – Eric Lippert Dec 01 '11 at 16:06

5 Answers5

11

I believe that you meant to say an object that has already been GC'd, not disposed. Well that can't happen; the MultiCastDelegate (the EventHandler) maintains a reference to the object via its subscribed method, i.e., it can't be GC'd until the handler is removed.

Dispose() has nothing to do with a method not being available, it is a pattern used to cleanup native resources, i.e., resources that cannot be handled by the GC.

The object itself is still alive and well, though it may throw an exception if you call a method that depended upon that native resource (depends on implementation of course. The point is that the object still exists as does the method).

There is nothing magical happening when you call Dispose(). I could easily whip up a class that implements IDisposable and has a completely empty Dispose() method. Call it all you will, it does nothing and does not change the state of the object in any way.

Ed S.
  • 122,712
  • 22
  • 185
  • 265
  • Just to clarify, it is not that I believed that Dispose will do something, but that GC will run. I totally forgot that the event will act as a strong root and prevent GC on that object. – Alex K Nov 30 '11 at 22:31
2

Everything Ed S. said is accurate. I want to elaborate, that the behavior you will see will be dependent on the function/event handler's implementation. It may or may not throw an exception and it may or may not behave strangely.

Dispose does nothing magical that would make the object inaccessible. Dispose generally just does some things like if one of the fields/properties was a File handle, then it would call dispose in turn on that file handle to release the resource immediately(see http://blogs.msdn.com/b/kimhamil/archive/2008/11/05/when-to-call-dispose.aspx)

If the event handler attempted to do something with that file, then an exception might be thrown.

It is possible though that the code for that event handler just deals with member fields like ints and strings and thus might behave as normal.

It is also possible that the person who wrote that class/function has an explicit if(Disposed){throw blah; }, so that if you attempted to call the function it would throw an exception telling you that operation is invalid on a disposed instance.

It is important to unsubscribe from events, else your object will never be GC'd.

AaronLS
  • 37,329
  • 20
  • 143
  • 202
1

I believe that in most situations that you should not catch any exceptions thrown by subscribers. You truly can't be expected to handle every possible exception that can be thrown by client code. In short, I think the fail fast principle should be applied here.

ChaosPandion
  • 77,506
  • 18
  • 119
  • 157
  • Exactly, this is a problem with the object owning the handler and not the object raising the event. It's up to the event handler to deal with this properly. – Dan Bryant Nov 30 '11 at 22:07
1

It's up to the object to decide what should happen if it's already been Disposed (it can still execute code in a 'Disposed' state, as Dispose is just something you voluntarily call to tell the object to clean itself up.) I have seen this cause bugs during application shutdown before, where events were being handled on another thread while the object was being cleaned up; the event handlers were not written properly to prevent this race condition. It's worth watching out for.

Dan Bryant
  • 27,329
  • 4
  • 56
  • 102
1

This code always works...

var ev = NotifyPropertyChanged;
if (ev != null)     
    ev(this, new PropertyChangedEventArgs("Foo"));

... even if another thread unsubscribes the event handler between the first and the last line. This is because delegates are immutable. Each time event handlers are added or removed, a new multicast delegate is created. Therefore you can treat them like value types. So ev will never be changed after it has been assigned to in the first line.

Olivier Jacot-Descombes
  • 104,806
  • 13
  • 138
  • 188