views:

137

answers:

4

Note: I edited this question to make it easier for other people with the same problem to get help here. To see the original question that fits better with some of the answers, check the edit history.

In a project I have a ExecutionManager class that can contain multiple ExecutionSlot's instances. The ExecutionSlot class have several public event fields like this:

public event EventHandlers.ObjectEventHandler<IPlugin> ExecuteCompleted;

For each of these events there is a matching event on ExecutionManager. The desired behavior is that every time on of the ExecutionSlot's raises an event, the matching event is also raised on the containing ExecutionManager.

The implmented solution was that whenever an ExecutionSlot was added to an ExecutionManager the ExectionManager would add its own events to the ExecutionSlot's like this:

executionSlot.ExecuteCompleted += ExecuteCompleted;

There is no need yet to remove an ExecutionSlot, so the events are never removed either.

The problem is that the event on ExecutionManager is not being raised. After confirming that an event was being reaised by an ExecutionSlot I found out that changing the above line to the following fixed the problem:

executionSlot.ExecuteCompleted += (sender, eventArgs) => ExecuteCompleted(sender, eventArgs);

And I could not figure out why, so my question was, what the difference was.

The reason for this difference was that the first adds the current listeners of the ExecutionManager's event to the ExecutionSlot's event. So any listeners added later will not be called when the event is raised. In contrast the latter solution uses a lambda to raise the ExecutionManager's event, which means that the listeners at the time of the event will be called.

The underlying reason for the first solution to fail, is that delegates are immutable. So when you add a new delegate to and event, you are actually creating a new delegate that contains the existing delegates and the added. So any reference to the delegates made before will not contain the newly added delegate.

+4  A: 

One idea... perhaps there is somewhere in your code where you are doing:

executionSlot.ExecuteCompleted -= ExecuteCompleted;

which would unsubscribe the event if you use your original subscription syntax, but would not remove it once you made your change.

JoelFan
That can't do it, won't generate the same delegate object instance. The Target is different.
Hans Passant
What? I think we are saying the same thing! ???
JoelFan
Thanks for the suggestion, unfortunatly there is no unsubscription. But one of the reasons I would prefer the latter code to work is that I might need that in a later version of the program, and I really would prefer not having to write out the handler function, since there are more than a few events. I will try and but a bit more info in the question.
Lillemanden
+4  A: 

EDIT: This answer was assuming that ExecuteCompleted was a method. As it's actually a field, that changes things completely. I'll leave this answer here for the sake of posterity.

The first version adds an event handler with a delegate created from an autogenerated method which in turn just calls ExecuteCompleted. It's a bit like this:

private void <>AutogeneratedMethodWithUnspeakableName(object sender, EventArgs e)
{
    ExecuteCompleted(e);
}
...
executionSlot.ExecuteCompleted += <>AutogeneratedMethodWithUnspeakableName;

The second version adds an event handler with a delegate created directly from the ExecuteCompleted method.

Basically the first form is one extra level of redirection. This wouldn't usually make any difference, except for unsubscription as JoelFan mentioned. I would guess that's the problem.

The class raising the event could reflect over the attached handlers and look at the method names, reacting differently in this particular case - but it's very unlikely.

Jon Skeet
Thanks for the suggestion, unfortunatly there is no unsubscription. But one of the reasons I would prefer the latter code to work is that I might need that in a later version of the program, and I really would prefer not having to write out the handler function, since there are more than a few events. And I am not using reflection in the class raising the event either. I will try and but a bit more info in the question.
Lillemanden
@Lillemanden: If you could come up with a short but complete example which demonstrates the problem, I'm sure we'll be able to sort it out.
Jon Skeet
@Jon Skeet: I am pretty sure Protron nailed it. Since delegates are immutable, a new multicast delegate is created every time you use "+=". So when something subscribes to the last event in the chain a new delegate instance is created, but that instance is not added to the executionSlot.ExecuteCompleted event. At least that is AFAIK, hope it made sense.
Lillemanden
@Lillemanden: I'd been assuming that ExecuteCompleted was a method in your code. Is it not? If you could produce a short but complete program which demonstrates this, it would really help...
Jon Skeet
It is an event field. I'll add some code to question after lunch ;)
Lillemanden
@Lillemanden: Okay, in that case it all makes sense.
Jon Skeet
A: 

I believe what is happening here is that some sort of temporary object is being created in the first example with an empty event handler that is being called, but does nothing.

The second example, which you say works, is an event handler on your object with the real code. Not entirely sure what's going on there though, but that's my best guess.

Certainly the first example smells bad anyway, since it uses lambda expressions to obfuscate the meaning with no real added value.

Nick
It's probably not creating an object, actually - I'd expect the compiler to add an instance method to the current class, as it doesn't capture anything other than "this".
Jon Skeet
+2  A: 

look at this other post on stackoverflow

b += (s, e) => a(s, e);

is not the same as

b += a;

It appends the current contents of a to b, so if later more handlers enlist with a, this will not cause them to be called when b is fired

Protron
That's where a and b are variables - in this case it's the name of a method, so it's not actually capturing anything other than the "this" reference (which would be implicitly captured in the second form anyway).
Jon Skeet
Frig, you are absolutly right. Stupid immutable delegates ;)That explains it perfectly, the objects hooking up on the last event in the chain later would are not called. Thanks
Lillemanden
Okay, I'd been *assuming* that ExecuteCompleted was a method. If it's actually a variable (e.g. via a field-like event) then that does indeed make sense. Another example where having a reasonably complete example would have helped :)
Jon Skeet
Yes, it is indeed an event field. I'll add some code so other people can use this question as well.
Lillemanden