views:

1168

answers:

2

I'm using a SynchronizationContext to marshal events back to the UI thread from my DLL that does a lot of multi-threaded background tasks.

I know the singleton pattern isn't a favorite, but I'm using it for now to store a reference of the UI's SynchronizationContext when you create foo's parent object.

public class Foo
{
 public event EventHandler FooDoDoneEvent;

 public void DoFoo()
 {
  //stuff
  OnFooDoDone();
 }

 private void OnFooDoDone()
 {
  if (FooDoDoneEvent != null)
  {
   if (TheUISync.Instance.UISync != SynchronizationContext.Current)
   {
    TheUISync.Instance.UISync.Post(delegate { OnFooDoDone(); }, null);
   }
   else
   {
    FooDoDoneEvent(this, new EventArgs());
   }
  }

 }
}

This didn't work at all in WPF, the TheUISync instances UI sync (which is feed from the main window) never matches the current SynchronizationContext.Current. In windows form when I do the same thing they will match after an invoke and we'll get back to the correct thread.

My fix, which i hate, looks like

public class Foo
{
 public event EventHandler FooDoDoneEvent;

 public void DoFoo()
 {
  //stuff
  OnFooDoDone(false);
 }

 private void OnFooDoDone(bool invoked)
 {
  if (FooDoDoneEvent != null)
  {
   if ((TheUISync.Instance.UISync != SynchronizationContext.Current) && (!invoked))
   {
    TheUISync.Instance.UISync.Post(delegate { OnFooDoDone(true); }, null);
   }
   else
   {
    FooDoDoneEvent(this, new EventArgs());
   }
  }

 }
}

So I hope this sample makes enough sense to follow.

A: 

Rather than compare to the current one, why not just let it worry about it; then it is simply a case of handling the "no context" case:

static void RaiseOnUIThread(EventHandler handler, object sender) {
    if (handler != null) {
        SynchronizationContext ctx = SynchronizationContext.Current;
        if (ctx == null) {
            handler(sender, EventArgs.Empty);
        } else {
            ctx.Post(delegate { handler(sender, EventArgs.Empty); }, null);
        }
    }
}
Marc Gravell
I forget why this fails, but I know I tried this already. I can't remember if it was winform test app, the unit tests or the WPF app that broke, but this solution was incompatible with at least one of those techs.
Joel Barsotti
This code does not work as SynchronizationContext.Current should be "captured" on UI thread, while RaiseOnUIThread can be called on any other thread.
+6  A: 

The immediate problem

Your immediate problem is that SynchronizationContext.Current is not automatically set for WPF. To set it you will need to do something like this in your TheUISync code when running under WPF:

var context = new DispatcherSynchronizationContext(
                    Application.Current.Dispatcher);
SynchronizationContext.SetSynchronizationContext(context);
UISync = context;

A deeper problem

SynchronizationContext is tied in with the COM+ support and is designed to cross threads. In WPF you cannot have a Dispatcher that spans multiple threads, so one SynchronizationContext cannot really cross threads. There are a number of scenarios in which a SynchronizationContext can switch to a new thread - specifically anything which calls ExecutionContext.Run(). So if you are using SynchronizationContext to provide events to both WinForms and WPF clients, you need to be aware that some scenarios will break, for example a web request to a web service or site hosted in the same process would be a problem.

How to get around needing SynchronizationContext

Because of this I suggest using WPF's Dispatcher mechanism exclusively for this purpose, even with WinForms code. You have created a "TheUISync" singleton class that stores the synchronization, so clearly you have some way to hook into the top level of the application. However you are doing so, you can add code which creates adds some WPF content to your WinForms application so that Dispatcher will work, then use the new Dispatcher mechanism which I describe below.

Using Dispatcher instead of SynchronizationContext

WPF's Dispatcher mechanism actually eliminates the need for a separate SynchronizationContext object. Unless you have certain interop scenarios such sharing code with COM+ objects or WinForms UIs, your best solution is to use Dispatcher instead of SynchronizationContext.

This looks like:

public class Foo 
{ 
  public event EventHandler FooDoDoneEvent; 

  public void DoFoo() 
  { 
    //stuff 
    OnFooDoDone(); 
  } 

  private void OnFooDoDone() 
  { 
    if(FooDoDoneEvent!=null)
      Application.Current.Dispatcher.BeginInvoke(
        DispatcherPriority.Normal, new Action(() =>
        {
          FooDoDoneEvent(this, new EventArgs()); 
        }));
  }
}

Note that you no longer need a TheUISync object - WPF handles that detail for you.

If you're more comfortable with the older delegate syntax you can do it that way instead:

      Application.Current.Dispatcher.BeginInvoke(
        DispatcherPriority.Normal, new Action(delegate
        {
          FooDoDoneEvent(this, new EventArgs()); 
        }));

An unrelated bug to fix

Also note that there is a bug in your original code that is replicated here. The problem is that FooDoneEvent can be set to null between the time OnFooDoDone is called and the time the BeginInvoke (or Post in the original code) calls the delegate. The fix is a second test inside the delegate:

    if(FooDoDoneEvent!=null)
      Application.Current.Dispatcher.BeginInvoke(
        DispatcherPriority.Normal, new Action(() =>
        {
          if(FooDoDoneEvent!=null)
            FooDoDoneEvent(this, new EventArgs()); 
        }));
Ray Burns
Regardless of when you check unless your actually locking something I think you could always have that problem.And that's a threading issue that can't be fixed in your class, it has to be handled in the class that is wiring into your event (or unsubcribing from your event), although I would have to provide a locking object that the subscriber could use and I could use to make sure the events are syncrhornized.
Joel Barsotti
Yes and no. "public event ..." has threading all by itself issues if you assume it may be called free-threaded. If two threads both subscribe to the same event at the same time, they may not both get into the invocation list. So the assumption is always that you have some threading protocol in mind for setting the event in the first place. The most common such protocol is to set such events only on the UI thread, in which case the bug fix I gave works reliably whereas the original code does not. On the other hand, if a different protocol is intended you may need explicit locking.
Ray Burns