Thread overkill with async/await

Question

Consider the following code.

public async void Connect()
{
 bool success = await ConnectAsync();
 if (success) NotifyUserOfSuccess();
}
public async Task<bool> ConnectAsync()
{
 var t = await Task.Run(()=>
 try {
 ConnectSynchronously();
 } catch (Exception e)
 {
 return false;
 }
 return true;
 );
}

When analyzing this code, I come to the conclusion that I potentially am kicking off three threads. Assume that both await's will not already be completed when called, then the first thread is kicked off by the await ConnectAsync(); which in turn kicks off the await Task, and the Task.Run will kick off the function inside in a third thread. I really only need one thread, namely the one in Connect().

How can I improve the design?

As Connect() contains the await keyword, shouldn't it really be called ConnectAsync() regardless of its return value?

• DaveShaw: This question appears to be off-topic because it is asking for a Code Review.
• lejon: How can I demonstrate the problem without posting some code?!

Answer 1

You have a wrong assumption here. await never starts a new thread! Strictly speaking even Task.Run might not start a new thread since it uses the thread pool.

See this answer for more details on await: https://stackoverflow.com/questions/16978904/async-programming-control-flow/16980183#16980183

• lejon: I see. I was reading blog.stephencleary.com/2012/02/async-and-await.html and I guess I was confused by the statement: "If "await" sees that the awaitable has not completed, then it acts asynchronously. It tells the awaitable to run the remainder of the method when it completes, and then returns from the async method."

Answer 2

Your code will still run synchronously. In ConnectAsync() you await the Task.Run in effectively you are running it in sync with the rest.

Since you are already awaiting ConnectAsync you should return the task in ConnectAsync() and don't mark the function as async:

public async void Connect()
{
 bool success = await ConnectAsync();
 if (success) NotifyUserOfSuccess();
}
public Task<bool> ConnectAsync()
{
 return Task.Run(()=>
 try {
 ConnectSynchronously();
 } catch (Exception e)
 {
 return false;
 }
 return true;
 );
}

Now you run the anonymous function in async because it is wrapped in a task. You then can await this task to be completed as you do in Connect.

---

To answer your comment about function naming:

Because you have Connect(), ConnectAsync() and ConnectSynchronously(). I would rewrite your code to make the ConnectAync disapear and rename Connect to TryConnect to remove some confusion. But this is my take, it is always up to your own programming style.

public async void TryConnect()
{
 bool success = await Task.Run(()=>
 {
 try {
 ConnectSynchronously();
 } catch (Exception e)
 {
 return false;
 }
 return true;
 });
 if (success) NotifyUserOfSuccess();
}

---

Update:

obj.ToString() also doesn't block the thread. But should it be called obj.ToStringAsync()? But you bring a somewhat valid point. You could rename it as ConnectAsync() but I called it TryConnect because it not only connects, but it also responded to the user if successful. Hence, you try to connect. But again, it is you personal flavor. Just as long as the method name makes sence. TryConnect reflects well what the method does, as does ConnectAsync. As long as you dont call it DoStuff() because that doesnt reflect what the method does.

• lejon: Isn't there still two thread involved? The one started by await and the one by Task.Run(..)?
• SynerCoder: in total there will be 2 threads, 1, the main thread. and the other the task.run. async and await is used to make sure the main thread isn't frozen for the duration of the task. What in your case contains the long running ConnectSynchronously(). Because the use of async and await ConnectSynchronously() is executed in a different thread and the main thread isn't frozen in the mean time.
• lejon: Ok. I get it. What is your take on the naming of the methods?
• lejon: Shouldn't the method reflect that calling it will not block the calling thread? I.e. TryConnect should be renamed TryConnectAsync?
• SynerCoder: @lejon, see update

Answer 3

There are currently two threads used: one for Task.Run and one for everything else.

To improve the code, get rid of ConnectSynchronously and replace it with whatever truly-asynchronous connection mechanism you have, e.g.:

public Task ConnectAsync()
{
 return Task.Factory.FromAsync(BeginConnect, EndConnect, null); // or whatever
}
public async Task<bool> TryConnectAsync()
{
 try
 {
 await ConnectAsync();
 return true;
 }
 catch
 {
 return false;
 }
}

Then you only have one thread.