Is it wrong or bad practice to use Task.Run inside a void function?

Question

I am trying to create a type of "fire and forget" function where I don't care about the completion or the result of the function. In order to do that I am using Task.Run() in the body of that function so I can run it concurrently and return from it right away.

I think its important to mention that this runs is a IIS application.

Something like this:

void ReallyImportantFunction()
{
    //Do really important work
        NotSoImportantWork("some data");
    //Do really important work

}

void NotSoImportantWork(string data)
{
    Task.Run(() =>
    {
        try
        {
            //Do something here with the data   
        }
        catch (Exception ex)
        {
            //Log exception do not throw

        }


    });
}

https://blog.stephencleary.com/2013/11/taskrun-etiquette-examples-using.html — Daniel A. White, Jul 16 '19 at 14:55
https://stackoverflow.com/questions/1018610/simplest-way-to-do-a-fire-and-forget-method-in-c — Daniel A. White, Jul 16 '19 at 15:26

Tom Case · Accepted Answer · 2019-07-16T17:52:51.663

It's hard to answer this question without a bit more information, but I think you're misunderstanding the use of Task.Run() and asynchronous code. If it's unimportant work, and you don't care about the result or its completion, you might ask if you need to be running it here in the first place. Secondly, Task.Run() is typically used in order to execute long-running code that might otherwise block the UI thread. If you're making a DB call or writing to the File System, it could make sense to put this logic in a Task. An example being

void ReallyImportantFunction()
{
    try
    {
        //Do really important work
        Task.Run(() => NotSoImportantWork("some data"));
        //Do really important work

    }
    catch (Exception ex)
    {
        / Handle the exception
    }
}

void NotSoImportantWork(string data)
{
    //Do something here with the data   
}

However this exposes you to data races if you actually do ever use whatever you're computing, or if your computation ever has any side-effects or mutations. If what you're saying is true and you never do look at the result, I would probably ask why it's in the method to begin with. Why compute really unimportant things right in the middle of important ones if you're never going to use them? If you are using the result, or mutating something you need, it makes sense for your code to properly await the result.

async void ReallyImportantFunction()
{
    try
    {
        //Do really important work
        var myResult = await IOIntensiveWork("some data");
        //Do really important work
    }
    catch (Exception ex)
    {
        // Handle the exception
    }
}

Task<MyResult> IOIntensiveWork(string data)
{
    //Do something here with the data   
}

It's usually cleaner if you can avoid async void methods too, though there's nothing necessarily wrong with it. Just remember to always check for exceptions.

Edit: Based on what you've written in the comments, I think it makes sense to wrap your NotSoImportantWork() in a task. This is not so much because it is unimportant, but rather because you are apparently making a network call, and there are a variety of problems that can occur while communicating with a service outside your control.

So you could just Task.Run(() => NotSoImportantWork("...")); from ReallyImportantFunction(), but be mindful of where you're catching exceptions and where your code potentially exits if one should occur. I would probably prefer to launch the Task.Run() there rather than in NotSoImportantWork(), because you could potentially use Task.Run(...).ConfigureAwait(false); to squeeze that last bit of performance out of your code (this largely depends on how your code is structured).

Depending on where you're launching your task from, you can use ...ConfigureAwait(false); to say that you don't need to the task to return to the current context when the operation completes. This can sometimes give performance improvements. Try and use it whenever it makes sense. For more information, have a look at this: When correctly use Task.Run and when just async-await

Without more of a context to your code, it's hard to go into much more detail than this. Hope it helps!

The NotSoImportantWork is a network call to a message broker hosted on another machine that logs some information about the data that was processed. That information is updated every time a new data package comes in, so if something goes wrong I really don't care, because the next package will update it anyways. — Fabio S., Jul 16 '19 at 16:00
Couple more points. The performance of the ReallyImportantFunction() is crucial to our business, so adding as close to 0ms as possible to its execution is ideal. And the NotSoImportantWork() is called from multiple places, that's why I am adding the Task.Run to its body instead of duplicating on the outside. — Fabio S., Jul 16 '19 at 16:32
I've just edited my answer to include a couple suggestions. Have a look and tell me what you think. Hope it helps! — Tom Case, Jul 16 '19 at 17:53

Is it wrong or bad practice to use Task.Run inside a void function?

1 Answers1