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?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 29, 2013 at 16:18
\$\endgroup\$

1 Answer 1

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.

answered Jul 30, 2013 at 1:30
\$\endgroup\$
3
  • \$\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\$ Commented Jul 30, 2013 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\$ Commented Jul 30, 2013 at 19:37
  • \$\begingroup\$ lock(bridge)... thank you. Or I have to repair bridge itself. I'll +1 your answers when I can. \$\endgroup\$ Commented Jul 30, 2013 at 19:42

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.