Delegates and Events confusion

[Rhy Mednick]

I have a class (let's call it ClassA) that I've written which has events.
In another class (let's call it ClassB) I create a collection of ClassA
objects. In a third class (ClassC) I create a reference to some of the
ClassA objects created in ClassB. In ClassC I hook into the ClassA events
with a foreach loop so that I hook each object. The code is something like
this:
class ClassC {
void SomeMethod()
{
foreach (ClassA item in ClassACollection)
{
item.MyEvent += new EventHandler(item_MyEvent);
}
}
}

Objects of type ClassC keep getting created and deleted, but the objects of
ClassA that it references stay in memory. Therefore, every time I load a
new instance of ClassC the event gets hooked again. What I'm seeing is that
I fire the event once but I end up getting it raised in the code that
responds to it (item_MyEvent method) for every time it was added when I only
want it to get caught once.

How can I be sure that I only add the handler to the event once for each
item? Since the code is in ClassC it's not allowed to inspect the
ClassA.MyEvent to see if something's already hooked to it.

I hope this makes sense. Creating a repro case wouldn't make sense here
because it's my application logic that's causing the problem.

Thanks.

 

[J.Marsch]

Are you unsubscribing the event when you remove the instances of Class C
from memory?

If you subscribe to an event and do not unsubscribe, the subscriber will be
held in memory until the event publisher (class A) is collected.

Here's why:
A delegate has a member called Target. Target is a reference to the
subscriber (Class C). When you add the event, the publisher holds a
reference to the delegate which in turn holds a reference to the subscriber,
so C is held in memory. The graph looks like this:

A ---> EventDelegate ---->C

So, the reason that you are getting multiple event fires is that you have
more instances of C in memory than you think that you do.

What you probably want to do is to keep track of all of the event
subscriptions that C makes and have C implement IDispose. When you are
ready to kill C, call its Dispose method. Inside the Dispose, unsubscribe
from all of the events that you are subscribed to.

 

[Rhy Mednick]

Thanks. I figured that I needed to unsubscribe in some way but since I
wasn't storing an internal reference to the subscriber. I added the code
like the following and it solved the problem but I don't really understand
why:

item.MyEvent -= new EventHandler(item_MyEvent);

This just seems really odd to me. If the += assignment adds a refernece to
a new event handler it seems that handler would be a different object than
the one created with the new operator in the -= assignment. I could see
something like this working:
EventHandler eh = new EventHandler (item_MyEvent);
item.MyEvent += eh;
....then later...
item.MyEvent -= eh;
I can see that working because it's the same object, but when I use the new
keyword twice don't I have two unique references?

- Rhy

 

[Guest]

It IS confusing that you're *subtracting* a *new* object, but that IS the correct syntax. Remember that the 'new' event handler that you're subtracting is actually (in all probability) a reference to a specific instance's member function, so it's basically just saying 'this member function of this instance doesn't want to receive this event any more.' To tell the compiler that, all it needs is *ANY* object whose type is a delegate to that member function, and it will understand - presumably this was the least confusing (albeit more than slightly lateral) way of implementing this syntax.

 

[Jon Skeet [C# MVP]]

It IS confusing that you're *subtracting* a *new* object, but that IS
the correct syntax. Remember that the 'new' event handler that you're
subtracting is actually (in all probability) a reference to a
specific instance's member function, so it's basically just saying
'this member function of this instance doesn't want to receive this
event any more.' To tell the compiler that, all it needs is *ANY*
object whose type is a delegate to that member function, and it
willunderstand - presumably this was the least confusing (albeit more
than slightly lateral) way of implementing this syntax.

Bear in mind, however, that there are two parts to a delegate - the
method itself, and the target object. So doing

Foo f = new Foo();
x.SomeEvent += new SomeDelegate(f.Something);
x.SomeEvent -= new SomeDelegate(f.Something);

will add and then remove the handler.

x.SomeEvent += new SomeDelegate (new Foo().Something);
x.SomeEvent -= new SomeDelegate (new Foo().Something);

will fail to remove the handler it's added, as the two delegates in
question aren't equal.

 

[J.Marsch]

To answer that question, we can fire up Reflector
http://www.aisto.com/roeder/dotnet/ and look at the definition of the
Delegate class. In it, we find that the the Equals() method has been
overridden. Two delegates are considered equal if they both target the same
object instance and the same method within that object instance.

So:
event1 = new EventHandler(item_MyEvent);
event2 = new EventHandler(item_MyEvent);

?event1.Equals(event2); // true

When you execute this:
item.MyEvent -= new EventHandler(item_MyEvent);

The publisher goes looking for a delegate that it is holding that is equal
to the one that you passed, and it finds the one that you passed in the +=
statement.

 

[J.Marsch]

Interesting. I was just playing around with the Equals function in the
snippet compiler (http://www.sliver.com/dotnet/SnippetCompiler/) , and I
think that I may have spotted a bug -- I don't want to jump to conclusions,
though.

Correct me if I'm wrong, but if you override Equals, you should also
override GetHashCode (you actually get a compiler warning if you don't -- I
know this part is right). Shouldn't the results of GetHashCode be
consistent with Equals -- two items that are "equal" should return the same
hash code, but two items that are not equal should return different hashes?

I was pretty sure that this is how it was supposed to work. Maybe I'm
wrong. What I find is that it is possible to have 2 delegates where
Equals() is false, but the Hashes returned from GetHashCode are the same.

This can happen if you have 2 different delegates pointing to the same
method name in 2 different object instances. In Reflector, it appears the
Equals() takes into account both the target and the method pointer, but
GetHashCode only takes into account the method pointer (which will be the
same for all instances of the same object, as only the fields are copied in
memory).

Below is a short but complete program to demo (.Net framework v 1.1):

using System;
using System.Collections;

public class MyClass
{
public static void Main()
{
EventTarget handler1 = new EventTarget();
EventTarget handler2 = new EventTarget();
System.EventHandler event1 = new System.EventHandler(handler1.Handler);
System.EventHandler event2 = new System.EventHandler(handler2.Handler);

// first test equal -- should be false
WL("Event1.Equals(Event2): {0}", event1.Equals(event2));
// second test -- hashcode
int hash1 = event1.GetHashCode();
int hash2 = event2.GetHashCode();
WL("Event1 Hash: {0}, Event2 Hash: {1} Hash1 == Hash2: {2}", hash1, hash2,
hash1 == hash2);
RL();
}

private static void WL(string text, params object[] args)
{
Console.WriteLine(text, args);
}

private static void RL()
{
Console.ReadLine();
}

private static void Break()
{
System.Diagnostics.Debugger.Break();
}
}

public class EventTarget
{
public void Handler(object sender, System.EventArgs e)
{
}
}

 

[Jon Skeet [C# MVP]]

Interesting. I was just playing around with the Equals function in the
snippet compiler (http://www.sliver.com/dotnet/SnippetCompiler/) , and I
think that I may have spotted a bug -- I don't want to jump to conclusions,
though.

Correct me if I'm wrong, but if you override Equals, you should also
override GetHashCode (you actually get a compiler warning if you don't -- I
know this part is right). Shouldn't the results of GetHashCode be
consistent with Equals -- two items that are "equal" should return the same
hash code, but two items that are not equal should return different hashes?

No - two items which are equal should return the same hash code, but
two items which are not equal *may* return the same hash code.
Otherwise you couldn't *possibly* have a hash code for every string,
for instance.

Hash tables perform *better* if unequal objects return unequal hash
codes, but it's not necessary.

 

[J.Marsch]

Well that clears it up. I'd much rather be wrong than have a bug.

 

[Angelos Petropoulos]

No no no no no no, this is no reason to implement the IDisposable
interface!

When you implement the IDisposable interface, the design pattern
requires you to implement Finalize as well. The code inside Finalize
calls the Dispose method (it is in other words your failsafe in case
the client does not call Dispose). If you call Dispose manually,
GC.SuppressFinalization is called and the object is not placed in the
Finalization queue.

Now, not only do you not want to get into all this for every class
that subscribes to events, but implementing the Finalize method means
that every instance of that class is placed in the finalization list
of the GC so that it knows not to free up the memory that the object
is occupying before calling its Finalize method. Even if you do call
Dipose manually and you suppress the object's finalization, the
initial cost of placing the object in the finalization list is still
there.

The IDisposable interface should only be implemented when you are
dealing with unmanged resources, which if not freed would cause a
memory leak.

Regards,
Angelos Petropoulos

 

[Jon Skeet [C# MVP]]

No no no no no no, this is no reason to implement the IDisposable
interface!

When you implement the IDisposable interface, the design pattern
requires you to implement Finalize as well.

Which design pattern? Although it's *common*, you certainly don't
*have* to implement Finalize as well. If your class wraps another class
which implements IDisposable, but your class doesn't contain any
unmanaged resources *directly*, it makes sense to implement IDispose in
order to dispose of the internal object - but it doesn't make sense to
have a finalizer, as if the internal object is implemented properly,
*it* will have a finalizer which will be called anyway.

There are various examples of this in the normal class library, and
Richter mentions it in Applied Microsoft .NET Framework development.

 

[Angelos Petropoulos]

You are right, in the scenario you describe it wouldn't be necessary
to have a finalizer. I hadn't thought of that, thank you for pointing
it out.

Regards,
Angelos Petropoulos

 

[J.Marsch]

The pattern does not require that you implement Finalize and IDisposeable in
tandem. It merely advises that you do so. In this case, implementing a
finalize is unneccessary, and not even fruitful. You generally implement a
finalizer if your object holds unmanaged resources that must be
deterministically released when your managed object goes away. You
complement that finalizer with a Dispose so that the developer can call
dispose and bypass an expensive finalization. Here, we have a completely
different situation.

In the case in this thread, a finalizer would never be called, because if
you don't deterministically dispose this object (and unsubscribe the events)
it is _never eligible_ for collection (or finalization). Well, to be more
accurate: finalize would be called, but only when the event publisher is
elligible for collection. By that time, 1. The subscriber has lived too
long, and 2. there's no use in unsubscribing the events because the event
publisher is already gone.

Rather, by calling Dispose, you are allowing the object to become eligible
for collection. Again, if the object doesn't release its event
subscriptions, it will not even be eligible for collection until the event
publisher is eligible.

You could name the method something else (ReleaseEvents, or whatever), but
it's still fulfilling the same purpose as a Dispose. By naming it Dispose()
and implementing IDisposeable, you do two things:

1. You draw attention to other developers that this is a Disposeable object
( a known pattern), and so (hopefully) increase the likely hood that other
developers will call Dispose on the object
2. By implementing IDisposeable, you make the object compatible with the
Using() statement for automatic disposal when you are finished with it.