1
\$\begingroup\$

I have a method that calls a "bridge" for TraderMarketInfo. When info is received, it checks if info IsSet and raises an event. I want to make this method async. C# .NET4.0 VS2012.

Original method:

public void RequestMarketInfo()
{
    TraderMarketInfo marketInfo = bridge.GetMarketInfoAll(Data.Symbol);
    if (marketInfo.IsSet)
        OnMarketInfoUpdated(new MarketInfoUpdateEventArgs(Data.ConnectionId, marketInfo));
}

My attempt to make it async:

public void RequestMarketInfo()
{
    var task = Task<TraderMarketInfo>.Factory.StartNew(() => bridge.GetMarketInfoAll(Data.Symbol));
    task.ContinueWith(t =>
    {
        if (t.Result.IsSet)
            OnMarketInfoUpdated(new MarketInfoUpdateEventArgs(Data.ConnectionId, t.Result));
    });
}

Is it done right?

\$\endgroup\$
2
\$\begingroup\$

You need to be very careful when making code like this multithreaded. Your implementation will compile, and will execute the GetMarketInfoAll() method on another thread, but it may not do what you want. There are at least two things to consider:

  • Synchronization:
    The lambda function given to ContinueWith can (and often will) be executed on a different thread from the one that called RequestMarketInfo. The lambda function in your code invokes an event handler, this means that the code inside the event handler may need to synchronize any data it accesses, or you will end up with race conditions and other threading issues.

  • Thread Affinity:
    Similar to the first point, it may be necessary for you to ensure that the event handler is invoked on a certain thread. If any of the handlers for the OnMarketInfoUpdated event are updating a UI component in Windows Forms, then you will certainly need to perform those updates on the UI thread using Invoke.

You should also note that depending on the type of application you are developing, it may not actually help you to convert this to async. For example, web applications under heavy load are already using 1 thread per web request, and are often maxed out. However, I suspect you are using this inside a GUI and are trying to make the UI more responsive while it processes this long running query, in which case converting this to async is a good idea.

\$\endgroup\$
  • \$\begingroup\$ Thank you @Gordon Burgett. Bridge uses NamedPipes to communicate with third party app. The code above starts giving "Cannot connect to client" error in 50% of the cases. Event risen by OnMarketInfoUpdated causes different calculations on different classes but finally it updates UI (BeginInvoke). Except of the pipe error it works. I'm not sure is it (and other similar calls) creates several clients at a time and break the pipe? \$\endgroup\$ – Miroslav Popov Jul 30 '13 at 4:47
  • \$\begingroup\$ It is possible that your bridge class is not thread safe. You may try synchronizing the call to the method, ex. lock(bridge){ return bridge.GetMarketInfoAll(Data.Symbol); } \$\endgroup\$ – Gordon Burgett Jul 30 '13 at 19:37
  • \$\begingroup\$ lock(bridge)... thank you. Or I have to repair bridge itself. I'll +1 your answers when I can. \$\endgroup\$ – Miroslav Popov Jul 30 '13 at 19:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.