Which of these two methods is the correct one for running a CPU-intense long-running task in a WPF desktop app?

Question

In regards to async operations, I understand it's bad practice to use Task.Run in your implementation if you are exposing your code in a library to be consumed elsewhere as you should leave such decisions to the caller of your library. That information can be found in the 'duplicate' questions here. But that is not what I am asking. I am seeking clarification for our own code which is in a WPF desktop app and will never be exposed as a library for others to consume.

Additionally, our task takes 1-2 minutes of intense CPU-processing if that makes any difference.

My question is in our particular case, does it make sense to use a background thread to make our non-async function async, or do we simply make our function async, then call it with await?

Also, the long-running function doesn't do any async/await stuff itself. It's purely a linear, CPU-intensive task, and I don't care where it's run, only that its return value (and the progress) are reported back to the main UI.

That is what I'm seeking clarification on.

I'm using .NET 5.0 with C#9 in a WPF desktop app directly (i.e. this code is not in a library to be shared with others) if it matters.

Here's some (updated for clarity!) sample code illustrating this...

public static void Main() {

    // Local function to start the import
    // Note: I read I should be using `async void` and not 'async Task' here since
    // this is a top-level async statement and I understand you need this for proper
    // exception handling. Is that correct?
    async void startImport(){

        Debug.WriteLine($"Import starting... IsBackground: " +
            $"{Thread.CurrentThread.IsBackground}");

        var progress = new Progress<string>(message =>
            Debug.WriteLine($"Progress Update: {message}"));
            
        // Toggle the comments on these lines and the line defining
        // CPUIntenseSequentialImport to try it with and without Task.Run() 
        var result = await Task.Run(() => CPUIntenseSequentialImport(progress));
        //var result = await CPUIntenseSequentialImport(progress);

        Debug.WriteLine($"Finished! The result is {result} - IsBackground: " +
            $"{Thread.CurrentThread.IsBackground}");
    }

    startImport();

    Debug.WriteLine($"Import started... IsBackground: " +
        $"{Thread.CurrentThread.IsBackground}");
}

// Can you just mark this as async, then use await on it and not need Task.Run(),
// or because it's a long-running task, should you NOT mark it with async, and
// instead use Task.Start() so it gets its own thread from the pool?
static public int CPUIntenseSequentialImport(IProgress<string> progress) {
//static async public Task<int> CPUIntenseSequentialImport(IProgress<string> progress) {

    Thread.Sleep(1000);
    progress.Report($"First part of import done - IsBackground: " +
        $"{Thread.CurrentThread.IsBackground}");

    Thread.Sleep(1000);
    progress.Report($"Next part of import done - IsBackground: " +
        $"{Thread.CurrentThread.IsBackground}");

    Thread.Sleep(1000);
    progress.Report($"Final part of import done - IsBackground: " +
        $"{Thread.CurrentThread.IsBackground}");

    return 44;
}

When run as-is, you get this (Note the updates are not reporting from a background thread)...

Import starting... IsBackground: False
Import started... IsBackground: False
Progress Update: First part of import done - IsBackground: False
Progress Update: Next part of import done - IsBackground: False
Progress Update: Final part of import done - IsBackground: False
Finished! The result is 44 - IsBackground: False

...but when you swap the commented/uncommented lines, you get this (Note the updates are reporting from a background thread)...

Import starting... IsBackground: False
Import started... IsBackground: False
Progress Update: First part of import done - IsBackground: True
Progress Update: Next part of import done - IsBackground: True
Progress Update: Final part of import done - IsBackground: True
Finished! The result is 44 - IsBackground: False

Answer 1

There's actually a lot to unpack here, so I'll answer as best I can as I go along.

I understand it's bad practice to use Task.Start in your implementation if...

It's actually bad to use Task.Start in any code at all. There is one extremely obscure exception to that rule, but in general Task.Start just shouldn't be used at all. Task.Run is what you should be using to push work to a thread pool thread.

I am seeking clarification for our own code which is in a WPF desktop app and will never be exposed as a library for others to consume.

That is valuable information. At the end of the day, your own code is your own code, and even if you use things like Task.Start, that's up to you. On the other hand, good practices become good practices because people have discovered that they make code more maintainable. (At least, that's the theory I'm sticking with - cargo cults notwithstanding ;)

Specifically, even though you're not distributing your library on NuGet, your code may benefit from treating it kind of like a library. Even if it's not in an actual library (dll) project, the code would probably benefit from being treated as though it were in a separate layer. Separation of concerns and all that - the long-running processing code shouldn't know it's being invoked from a UI thread (and, given unit tests, it might not be).

does it make sense to use a background thread to make our non-async function async, or do we simply make our function async, then call it with await? Also, the long-running function doesn't do any async/await stuff itself.

I would say keep it synchronous, then. The only reason you would make it async is to unblock the UI thread. But this is the long-running processing code; why should it even know that there is a UI thread? To quote myself:

Back up to the original problem for a moment. What is the problem? The UI thread is blocked. How did we solve it? By changing the service. Who needs the asynchronous API? Only the UI thread. Sounds like we just seriously violated the “Separation of Concerns” principle.

The key here is that the solution does not belong in the service. It belongs in the UI layer itself. Let the UI layer solve its own problems and leave the service out of it.

On to the code.

public static void Main()

This is the first problem. Console apps behave quite differently than WPF apps when it comes to async because of the way await interacts with the SynchronizationContext. Since your actual app is a WPF app, it's far better to experiment with a sample WPF app than a sample Console app.

I read I should be using async void and not 'async Task' here since this is a top-level async statement and I understand you need this for proper exception handling. Is that correct?

async void is used for event handlers. In all other cases, avoid async void. It's not appropriate in a Console app, although it could be used in a WPF app.

Can you just mark this as async, then use await on it and not need Task.Run()

No. async does not start a new thread. When you compile this code, the compiler itself will give you a warning that the method will run synchronously.

should you NOT mark it with async, and instead use Task.Start() so it gets its own thread from the pool?

This is the best solution. Keep the synchronous method synchronous, and call it using Task.Run.

When run as-is, you get this (Note the updates are not reporting from a background thread)...

Yes, because everything is actually run synchronously. There's nothing asynchronous here at all, and if you tried this in a WPF app, your UI thread would freeze up.

...but when you swap the commented/uncommented lines, you get this (Note the updates are reporting from a background thread)...

Yes, because this is a Console application. Progress<T> captures SynchronizationContext.Current in its constructor, and uses that to marshal progress updates. Since this is a Console app, there is no SynchronizationContext, and each IProgress<T>.Report gets sent to a thread pool thread. Possibly out of order (Progress<T> really shouldn't be used in Console apps at all).

If you ran that in a WPF application, then the Progress<T> would capture the WPF SynchronizationContext and the progress reports would be sent to the UI thread.