0

I have a component, which can perform long async actions and can be destroyed before that actions are completed. In that case, I want just to stop running that actions and forget about them, without any exceptions. I have the next example:

private CancellationTokenSource destroyTokenSource = new CancellationTokenSource();

private async void RunExample()
{
    await LongAction().MuteOnDestroy(destroyTokenSource.Token);
    // ... actions that should not be performed with destroyed object
}

private async Task<bool> LongAction()
{
    // some very long actions
    return true;
}

// ...

public static class TaskExtension
{
    public static Task<T> MuteOnDestroy<T>(this Task<T> task, CancellationToken destroyToken)
    {
        var promise = new TaskCompletionSource<T>();
        task.ContinueWith(t =>
        {
            if (!destroyToken.IsCancellationRequested)
            {
                switch (t.Status)
                {
                    case TaskStatus.Canceled:
                        promise.SetCanceled();
                        break;
                    case TaskStatus.Faulted:
                        promise.SetException(t.Exception!);
                        break;
                    case TaskStatus.RanToCompletion:
                        promise.SetResult(t.Result);
                        break;
                }
            }
        }, TaskContinuationOptions.ExecuteSynchronously);
        return promise.Task;
    }
}

I have an assumption that the async method (in this example RunExample method) that never completes (destroyToken.IsCancellationRequested == true) can cause memory leaks but I am not sure because I don't know where the continuation of "forever waiting" method is stored.

Can it cause memory leaks?

Dem0n13
  • 766
  • 1
  • 13
  • 37
  • [async void](https://learn.microsoft.com/en-us/archive/msdn-magazine/2013/march/async-await-best-practices-in-asynchronous-programming#avoid-async-void) is intended for event handlers. Is the `RunExample` method an event handler? If not, is it possible to convert it from `async void` to `async Task`? – Theodor Zoulias Oct 14 '21 at 21:49
  • @TheodorZoulias, yes, it's possible – Dem0n13 Oct 15 '21 at 04:54
  • I might worth it to switch to `async Task`, if you have a way to manage and eventually `await` those tasks. Otherwise, if you would launch these tasks in a fire-and-forget manner, sticking to `async void` is preferable. At least in case of an error that app will crash instead of hang. – Theodor Zoulias Oct 15 '21 at 06:48

1 Answers1

0

Yes, you're absolutely causing leaks. The TPL is going to make sure that the GC doesn't clean up any in progress tasks, and all of those tasks are going to have a reference to their continuation delegates, which themselves will often have references to more objects. It would be a problem if these objects were allowed to be cleaned up by the GC, because that's all code that can, and likely will, eventually run. The whole point of the GC is that it won't clean up references to objects that may be accessed from code that could be executed.

The problem is that your attempt to wrap a task with a cancellation token never completes early if the cancellation token is cancelled before the task you are wrapping finishes. It waits until the operation finishes normally to mark the wrapped task as cancelled.

Fortunately, solving this problem properly is in fact simpler than your existing solution as you can take advantage of existing TPL tools to do most of the hard work in this situation.

The linked solutions will result in the task being marked as cancelled much sooner, so the long running task itself will still keep references to all of the objects its using alive until it finishes, but the continuations leveraging the cancellation token will be able to be be cancelled as soon as possible and free up whatever resources they're holding onto.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • In my environment (Unity) the component can be destroyed before the long task is complete, and I don't want to catch cancelled exceptions or check the state of component after task completion. I just want to cancel all continuations and forget. – Dem0n13 Oct 14 '21 at 21:03
  • @Dem0n13 Well, you have two choices, you can either use the suggested code to cancel the tasks as quickly as possible and let the continuations do whatever behavior is appropriate for them on cancellation or you can use your code and have the active tasks stay around and keep every Task object and every object in the reference tree of the tasks or continuations around. If you're unsure which is more of a problem for your application, by all means, do some performance testing of the two to see how much memory is consumed and whether it's a problem for you. – Servy Oct 14 '21 at 22:44