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?

This question appears to be off-topic because it is asking for a Code Review. — DaveShaw
– DaveShaw, Commented Jul 1, 2013 at 9:07
How can I demonstrate the problem without posting some code?! — lejon
– lejon, Commented Jul 1, 2013 at 9:10

Community · Accepted Answer · 2017-05-23 12:40:48Z

11

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

edited May 23, 2017 at 12:40

CommunityBot

1

answered Jul 1, 2013 at 9:15

cremor

2112 silver badges11 bronze badges

\$\begingroup\$ 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." \$\endgroup\$

lejon
– lejon

2013-07-01 09:18:44 +00:00
Commented Jul 1, 2013 at 9:18

Add a comment |

SynerCoder · Accepted Answer · 2013-07-01 09:09:56Z

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.

Isn't there still two thread involved? The one started by await and the one by Task.Run(..)? — lejon
– lejon, Commented Jul 1, 2013 at 9:13
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. — SynerCoder
– SynerCoder, Commented Jul 1, 2013 at 9:16
Ok. I get it. What is your take on the naming of the methods? — lejon
– lejon, Commented Jul 1, 2013 at 9:19
Shouldn't the method reflect that calling it will not block the calling thread? I.e. TryConnect should be renamed TryConnectAsync? — lejon
– lejon, Commented Jul 1, 2013 at 9:47

Stephen Cleary · Accepted Answer · 2013-07-01 10:24:17Z

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.

Stack Exchange Network

Thread overkill with async/await

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Thread overkill with async/await

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions