tags:

views:

438

answers:

4

I'm looking into options for doing asynchronous event dispatching in a component that has many subscribers to its events. In perusing the options, I ran across this example:

public event ValueChangedEvent ValueChanged;
public void FireEventAsync(EventArgs e)
{
    Delegate[] delegates = ValueChanged.GetInvocationList();
    foreach (Delegate d in delegates)
    {
        ValueChangedEvent ev = (ValueChangedEvent)d;
        ev.BeginInvoke(e, null, null);
    }
}

Beyond the older syntax (the sample was from .NET 1.1), it looks to me like this is a serious resource leak. There's no completion method, no polling for completion, or any other way that EndInvoke will be called.

My understanding is that every BeginInvoke must have a corresponding EndInvoke. Otherwise there are pending AsyncResult object instances floating around, along with (potentially) exceptions that were raised during the asynchronous events.

I realize that it's easy enough to change that by supplying a callback and doing an EndInvoke, but if I don't need to . . .

Handling the asynchronous exeptions is another matter entirely, and, combined with the need to synchronize with the UI thread (i.e. InvokeRequired, etc.) could very well tank the whole idea of doing these asynchronous notifications.

So, two questions:

  1. Am I correct in believing that every BeginInvoke requires a corresponding EndInvoke?
  2. Beyond what I've noted above, are there other pitfalls to doing asynchronous event notifications in Windows Forms applications?
A: 
  1. No. EndInvoke is only required if a return type is specified. Check this out:thread. Also, I posted this thread which is semi related.

  2. I really cant help you with that one! :-) sorry.

Mike_G
Unfortunately, the MSDN thread isn't definitive, but it gives me some pointers where I can track this down.
Jim Mischel
+1  A: 

A call to BeginInvoke() should be paired with a EndInvoke() but not doing it will not result in a resource leak. The IAsyncResult returned by BeginInvoke() will be garbage collected.

The biggest pitfall in this code is you are highly exposed to exceptions terminating the application. You might want to wrap the delegate invocation in an exception handler and put some thought into how you want to propagate the exceptions that happen (report the first, produce an aggregate exception, etc).

Invoking a deletage using BeginInvoke() will take a thread off the thread queue to start running the event. This means that the event will always fire off the main UI thread. This might make some event handler scenarios harder to handle (e.g. updating the UI). Handlers would need to realize they need to call SynchronizationContext.Send() or .Post() to synchronize with the primary UI thread. Of course all other multi-thread programming pitfalls also apply.

chuckj
Yeah, the requirement that clients (i.e. event subscribers) be prepared to handle notifications on pool threads could very well be a killer here. I'm thinking that clients should subscribe asynchronously if they want rather than having the control force it.
Jim Mischel
A: 

Welcome to the drawbacks of events, leakages, issues with BeginInvoke, delegates, pools, runtime and language shortcoming. Pure lack of scalability they pose..

Even with constant fixes to the thread pool idea, new dispatchers, new binding mechanisms, dependency properties, you still get massive CPU usage, and huge memory leaks. All those "benefits", for not managing memory(yet still so easy to leak many devs just walk confident in their C#; utter nonsense).

Doing your own mechanism would take all their pride as well as MS standard practices to the park. I'd go for it.

rama-jka toti
A: 

After thinking about this for a while, I came to the conclusion that it's probably a bad idea to do asynchronous events in Windows Forms controls. Windows Forms events should be raised on the UI thread. Doing otherwise presents an undue burden on clients, and possibly makes a mess with AsyncResult objects and asynchronous exceptions.

It's cleaner to let the clients fire off their own asynchronous processing (using BackgroundWorker or some other technique), or handle the event synchronously.

There are exceptions, of course. System.Timers.Timer, for example, raises the Elapsed event on a thread pool thread. But then, the initial notification comes in on a pool thread. It looks like the general rule is: raise the events on the same thread that got the initial notification. At least, that's the rule that works best for me. That way there's no question about leaking objects.

Jim Mischel