1

I have a slow proc gen function that runs when a user changes any parameters.

If a parameter is changed before it has completed, I want it to cancel the task and start a new one.

Currently I have it checking if a cancellation token is null, and if not requesting a cancellation before launching a new task.

public static async void Generate(myInputParams Input)
{
    SectorData myData;

    if (_tokenSource != null)
    {
        _tokenSource.Cancel();
        _tokenSource.Dispose();
    }

    _tokenSource = new CancellationTokenSource();
    var token = _tokenSource.Token;
    myData = await Task.Run(() => SlowFunction(Input, token));

    // do stuff with new data
}

This does work but it seems to me that it's possible for the new task to be run before the cleanup code and cancelation in the previous one have completed.

Is there a way I can guarantee the previous task is done before starting the new one?

EDIT: I had forgotten to pass the token to the SlowFunction in this example. That wasn't the issue with my real one it just happened when I was renaming things to make it easier to read.

Also typos

arcadeperfect
  • 137
  • 10
  • seems odd that `myData` is not defined here – Mark Schultheiss Aug 08 '22 at 23:34
  • 1
    it's a unity app – arcadeperfect Aug 09 '22 at 00:08
  • @MarkSchultheiss simply for brevity. I can include that data structure if it's relevant. – arcadeperfect Aug 09 '22 at 00:10
  • 3
    `async void` is bad. The only time you should do that is for event handlers. Otherwise always use `async Task`. – Enigmativity Aug 09 '22 at 00:52
  • 1
    You did not pass the `CancellationToken` to `SlowFunction`. It will probably not recognize this cancellation. [Stephen Cleary](https://blog.stephencleary.com/) wrote 4 excellent articles about cancellation. It's worth to read. Cancellation is cooperative so your `SlowFunciton` has to check for that. Btw: the function may throw an `OperationCancelledException` hat should be handled. Your `async void` method should also catch all exceptions because any may lead to an unhandled task exception and cause the whole applition to crash. – Sebastian Schumann Aug 09 '22 at 04:37
  • IMHO use `SemaphoreSlim` to ensure that the new task blocks until the prior task has exited. – Jeremy Lakeman Aug 09 '22 at 05:08
  • There are quite a few existing questions. Illustrate why yours is different here. Perhaps you just need a properly managed cancelation token? https://docs.microsoft.com/en-us/dotnet/standard/parallel-programming/task-cancellation re: https://stackoverflow.com/q/71608228/125981 and https://stackoverflow.com/q/10134310/125981 and https://stackoverflow.com/q/50232129/125981 and https://stackoverflow.com/q/72005686/125981 and https://stackoverflow.com/a/19005066/125981 OR perhaps a .Net 6 discussion can be found? https://andrewlock.net/cancelling-await-calls-in-dotnet-6-with-task-waitasync/ – Mark Schultheiss Aug 09 '22 at 11:59
  • Also the [convention](https://github.com/ktaranov/naming-convention/blob/master/C%23%20Coding%20Standards%20and%20Naming%20Conventions.md) for naming types in C# is PascalCase, and for naming parameters is camelCase. `Generate(MyInputParams input)` is correct. – Theodor Zoulias Aug 09 '22 at 13:20

1 Answers1

1

This does work but it seems to me that it's possible for the new task to be run before the cleanup code and cancelation in the previous one have completed.

Is there a way I can guarantee the previous task is done before starting the new one?

Sure: save the task in a variable (just like you're currently doing with _tokenSource), and await it before starting the new task. You'll probably want to await it in a try/finally block and ignore errors, at least if they're OperationCanceledException kinds of errors.

I'm assuming that this is a single-threaded UI context, where Generate is an event handler called on the UI thread. Otherwise, your class-level variables will need to be protected by mutexes and the async void should be removed.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810