BastianBlokland / componenttask-unity

Library to run dotnet's 'Task' and 'Task<T>' scoped to Unity components.
https://bastianblokland.github.io/componenttask-unity/
MIT License
21 stars 2 forks source link

How is the original task cancelling? #24

Open Dem0n13 opened 3 years ago

Dem0n13 commented 3 years ago

@BastianBlokland

If I use your library as:

        private void Start()
        {
            logger.Trace("Start", id, counter);
            this.StartTask(Async, options);
        }

        private async Task Async()
        {
            var localCounter = 0;
            while (Application.isPlaying)
            {
                await Task.Delay(2000);
                counter++;
                localCounter++;
                logger.Trace("Async", id, counter, localCounter);
            }
        }

What happens with the original Task when the object is destroyed? I see that you wrap the original task with CancellationSource, and use it for canceling on destroy but is it possible that the original task will hang somewhere and cause a memory leak? I can't reproduce the problem, but I can't understand what happens with original task? Does it save it's state somewhere?

BastianBlokland commented 2 years ago

Hi @Dem0n13,

When you call the StartTask extension method the first time on a new game-object it will create a MonoBehaviourTaskRunner component for managing tasks on that game-object.

The MonoBehaviourTaskRunner in turn makes a LocalTaskRunner that is responsible for keeping handles to the running tasks.

When the GameObject gets destroyed the MonoBehaviourTaskRunner disposes its LocalTaskRunner which then cancels all of the still running tasks and clears its runningTasks list. Which in turn means that no-one is holding onto the task-handles anymore and the memory will be cleaned up by the garbage collector.

Note: You can enable some diagnostic logging that should make some of this more clear also. Either globally with the ComponentTask.Config.GlobalDiagnosticsActive boolean or per task by giving the DiagnosticLogging flag in the TaskRunOptions when starting a task.

I hope that clears up how the tasks are destroyed.

BastianBlokland commented 2 years ago

One more thing that perhaps i didn't clearly explain: What makes this work is that it activates a custom System.Threading.SyncronizationContext when starting the task.

From LocalTaskRunner StartTask:

using (var contextScope = ContextScope.WithContext(this.context))
{
    var diagTracer = logger is null ? null : DiagTaskTracer.Create(logger, taskCreator);
    diagTracer?.LogInvoked();
    return this.WrapTask(taskCreator.Invoke(), diagTracer);
}

Note that ContextScope is a small helper that changes the current SynchronizationContext and then restores the old SynchronizationContext afterwards, so it won't mess up your previously active context.

This means when dotnet makes the Task<> it gets tied to the custom SyncronizationContext that belongs to that LocalTaskRunner instead of the default one. This allows us to manually decide when to execute tasks (See the Execute method in ManualSynchronizationContext but also means the workQueue is per task-runner (and not a global queue) and makes sure that the work-queue gets cleaned up when the LocalTaskRunner is destroyed.

Dem0n13 commented 2 years ago

@BastianBlokland

Thank you a lot for your explanations, a few more questions:

  1. I see that the only usage of the original task is wrapping this task with TaskHandle (In fact it is TaskCompletionSource). When I say wrapping, I mean creating of continuation:
    task.ContinueWith(TaskHandle.UpdateFromTask, handle, TaskContinuationOptions.ExecuteSynchronously)

    So we have two tasks here: the "original task" and its "continuation".

As I understand correctly, when we request cancellation via TaskCompletionSource (in TaskHadle.TryCancel) the "continuation" task enters the "Cancelled" state and will be never run because it has not been scheduled yet (https://stackoverflow.com/questions/48971316/when-i-use-cancelafter-the-task-is-still-running/48971668#48971668, https://blog.stephencleary.com/2015/01/a-tour-of-task-part-7-continuations.html). But what happens with the run "original" task at this moment? If it's an infinite loop like in the code above, I assume that it will be never completed and hang up somewhere inside .net task scheduling system on the last completed step/iteration. The only way to cancel the original task is to check CancellationToken.IsCancellationRequested or throw CancellationToken.ThrowIfCancellationRequested inside of the original task loop.

And I just want to ensure that the original task isn't saved anywhere inside of .net and doesn't cause any memory leaks, because I can't check its Task.Status and assume that it will ever be Completed.

  1. What's the purpose of ThreadStatic here https://github.com/BastianBlokland/componenttask-unity/blob/master/src/ComponentTask/Internal/ManualSynchronizationContext.cs#L13 ? Why don't we use the local List variable if we wan't just to copy the queue temporary?
BastianBlokland commented 2 years ago

Sorry if some of the following things are obvious, but this is an overview of how it works.

You can think of a task as dynamic number of synchronous steps, meaning every call to the task can create a new 'continuation' and it keeps calling the task until it no-longer wants any continuations and the task is done. Basically every 'await' call schedules a new continuation.

When we invocate the task the first time here (taskCreator.Invoke()) it immediately calls the first 'step' of the task (runs all you code until the first await).

using (var contextScope = ContextScope.WithContext(this.context))
{
    var diagTracer = logger is null ? null : DiagTaskTracer.Create(logger, taskCreator);
    diagTracer?.LogInvoked(data);
    return this.WrapTask<TOut>(taskCreator.Invoke(data, this.cancelSource.Token), diagTracer);
}

And when it encounters an await dotnet will schedule a continuation on the thing you await (Task.Delay for example). Those continuations will be 'tied' to the active System.Threading.SynchronizationContext which in our case is ManualSynchronizationContext. So when dotnet wants to continue the task it will call Send() on our SynchronizationContext, with a callback that will continue the 'rest' of the task.

Note: One important thing to realize is that dotnet can invoke Send() from any thread it wants, because when you do await foo().ConfigureAwait(false); it tells dotnet that it can run that task on its ThreadPool instead of on our SynchronizationContext, so the foo() can run on any thread and will call Send() on our SynchronizationContext when it wants to continue your task.

All of this also means that when you run a task that doesn't use await at all (or it returns before the first await) it is completely synchronous and from the perspective of the LocalTaskRunner it was never actually running.

You can see that handled here in WrapTask, it never actually gets added to runningTasks and a TaskHandle is never created.

if (task.IsCompleted)
{
    diagTracer?.LogCompletedSynchronouslyAsSuccess(default);

    return task;
}

Then about the TaskHandle<T>:

var handle = new TaskHandle<T>(this.exceptionHandler, diagTracer);
task.ContinueWith(TaskHandle<T>.UpdateFromTask, handle, TaskContinuationOptions.ExecuteSynchronously);

The TaskHandle has a few purposes but is probably less important then you think:

Note: The setting of the 'Cancelled' on the Wrapped tasks is only important if you wait on that task from another task in another task runner. Basically if GameObjectA would have a task that waits on the result of a task that runs on GameObjectB, then when GameObjectB is destroyed the task from GameObjectA needs to be informed that the task its waiting on is cancelled. The ExposeTask example has this scenario.

So its not involved in Cancelling the original task, and actually the original task is never cancelled it just ceases to exist when you dispose the LocalTaskRunner its part of. Reason why it ceases to exist is that any outstanding continuations it has are being pushed the SynchronizationContext of the now disposed LocalTaskRunner and will be garbage collected once all references to it are gone.

> I assume that it will be never completed and hang up somewhere inside .net task scheduling system on the last completed step/iteration.

The reason why it wont be hanging somewhere is that the continuations are scheduled on our SynchronizationContext and not the default dotnet one. So when we stop executing that SynchronizationContext it basically stops the 'chain' and the the task will be garbage-collected as no-one references it anymore.

> And I just want to ensure that the original task isn't saved anywhere inside of .net and doesn't cause any memory leaks, because I can't check its Task.Status and assume that it will ever be Completed.

Nothing should be keeping a reference to it (famous last words), you could verify by taking a weak reference to it and seeing if it gets GCed. A unit test like that would be cool :)

> Why don't we use the local List variable if we wan't just to copy the queue temporary?

Yep that would work also, this just avoids needing to allocate a new list every time.

Dem0n13 commented 2 years ago

Thank you for your explanations, I see! A unit test is a great idea.