0

It is a bit difficult for me to understand the following asynchronous usage. I have tested them locally and found that the actual result of both of them is the same, but I do not know in which scenarios the two should be used and whether there are potential problems?

The most basic requirement is that there is a property that does something. If true, it will start a thread (without interfering with the current thread) to perform a loop operation, and if false, it will close the loop operation.

public bool IsMaster
{
    get { return _isMaster; }
    set
        {
           _isMaster = value;

           if (_isMaster)
           {
              if (_httpTask == null || _httpTask.IsCompleted || _httpTask.IsFaulted)
              {
                 _httpTask = Task.Run(async () => await HandleHesApiRequest());
                 Logobject.LogDebug($"[HttpService]Master core,HTTP listenning started");
              }
           }
           else
           {
              while (true)
              {
                  //dispose resource
                 if (_httpTask == null || _httpTask.IsCompleted || _httpTask.IsFaulted)
                 {
                      Stop();
                      Logobject.LogDebug($"[HttpService]Worker core,HTTP listenning stopped");
                      break;
                  }

                 Thread.Sleep(2000);
               }
           }
       }
}

The asynchronous method is like this

public async Task HandleHesApiRequest()
{
    int relayCount = 2;

    while (true)
    {
        if (!IsMaster)
        {
           Logobject.LogDebug($"[HttpService]Changed to worker core,HTTP listenning stopping...");
           break;
        }
        else
        {
            //Do some job whether 'IsMaster==true'
        }

       await Task.Delay(xx)
     }

   Logobject.LogDebug($"[HttpService]Exit service...");

   await Task.CompletedTask;
}

The question is I don't understand the difference between

_httpTask =Task.Run(()=>HandleHesApiRequest()); 

and

_httpTask = Task.Run(async () => await HandleHesApiRequest());

The Sdk is .Net Core 3.1

Sir Rufo
  • 18,395
  • 2
  • 39
  • 73
  • `HandleHesApiRequest` seems to be a Asynchronous method, so why you use `Task.Run`? – Mohammad Aghazadeh Aug 04 '23 at 08:02
  • 3
    This is the case when [eliding async-await](https://blog.stephencleary.com/2016/12/eliding-async-await.html) is completely justifiable. – Guru Stron Aug 04 '23 at 08:16
  • 1
    `Thread.Sleep(2000);` is not something what you should do in properties. – Guru Stron Aug 04 '23 at 08:19
  • 1
    As @GuruStron points out, in this particular case there is no semantic difference, but there is a lot wrong with that code. You should rewrite the entire `IsMaster` property into a method which returns `Task`. There are also many other problems. It's not exception safe it's not thread safe and it's deeply surprising. – Aluan Haddad Aug 04 '23 at 09:32

3 Answers3

1
  • _httpTask =Task.Run(()=>HandleHesApiRequest()); : It is passing a synchronous delegate to Task.Run, which means that the Task.Run method will return a Task that represents the completion of HandleHesApiRequest.
  • _httpTask = Task.Run(async () => await HandleHesApiRequest());: It is passing an asynchronous delegate to Task.Run, which means that the Task.Run method will return a Task that represents the completion of the async lambda, which in turn awaits the completion of HandleHesApiRequest.
  • The only difference is that in the first one, you're creating another async lambda function to generate a Task while your Get method is already a Task, so you can use it directly.
Lee Dungx
  • 91
  • 4
1

For the given example there is no real difference. For a method that only calls another method it should make little practical difference if the returned task is awaited or not.

But you should probably use a timer instead. The "Loop around a sleep" is an anti pattern, while the "loop around a Task.Delay" is a bit better, it is probably still not the best way to do it.

Some basic rule of thumb are:

  1. If you are doing IO-operations, like reading from disk, or making http requests, use the Async pattern.
  2. If you are doing heavy computations, use a background thread, i.e. Task.Run
  3. If you are doing anything periodically you should use a timer.

There are cases where you would want to combine patterns, like using async pattern to wait for a slow computation, or a periodic timer together with async code. But that really depend on the specifics, and there is not enough details in the examples to provide any specific recommendation.

I would also recommend reading quite a bit about this topic. Multi threading and async/await are both fairly complicated topics. Dissecting the async methods in C# and There Is No Thread are a few good reads, but there are plenty of other good reasources out there.

JonasH
  • 28,608
  • 2
  • 10
  • 23
1

As mentioned, it is justifiable in this case to elide the async on the delegate, as it doesn't do anything useful.

However, you shouldn't be running such complex stuff inside a setter anyway, and certainly you shouldn't block looping waiting for the task just because it's a setter. Instead make a separate Set method that is async and await the task. this also allows exceptions on the task to bubble up correctly.

Furthermore, your if logic doesn't seem right, it seems the logic should be reversed when cancelling. And IsCompleted includes IsFaulted.

public bool IsMaster { get; private set; }

public async Task SetIsMaster(bool value)
{
    _isMaster = value;

    if (_httpTask?.IsCompleted ?? false)
    {
        await _httpTask;  // bubble up exceptions
        _httpTask = null;
    }

    if (_isMaster)
    {
        if (_httpTask == null)
        {
            _httpTask = Task.Run(HandleHesApiRequest);
            Logobject.LogDebug($"[HttpService]Master core,HTTP listenning started");
        }
    }
    else
    {
        // original logic seems incorrect
        if (_httpTask != null)
        {
            Stop();
            await _httpTask;
            _httpTask = null;
            Logobject.LogDebug($"[HttpService]Worker core,HTTP listenning stopped");
        }
    }
}
Charlieface
  • 52,284
  • 6
  • 19
  • 43