dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.5k stars 10.04k forks source link

Blazor: Deadlock when chaining TaskCompletionSource via ContinueWith #43364

Open JaleChaki opened 2 years ago

JaleChaki commented 2 years ago

Is there an existing issue for this?

Describe the bug

Hello, I'm trying to create a TaskCompletionSource chain in Blazor Server app. Each TCS.Task has an asynchronous continuation which triggers a next TCS (check the attached code below). After first completion of first continuation the entire circuit is blocked.

Expected Behavior

I attached code below. In this example I expect the following console output:

continuation 1 called
continuation 2 called

But I got output continuation 1 called and deadlock instead.

Steps To Reproduce

Issue can be reproduced with the following razor page. When you click at button deadlock occurs.

@page "/"

<button @onclick="MakeDeadlock">Make deadlock</button>

@code {
    void MakeDeadlock() {
        var tcs1 = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
        var tcs2 = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
        tcs1.Task.ContinueWith(
            _ => {
                Console.WriteLine("continuation 1 called");
                tcs2.SetResult();
            },
            CancellationToken.None,
            TaskContinuationOptions.RunContinuationsAsynchronously,
            TaskScheduler.FromCurrentSynchronizationContext()
        );
        tcs2.Task.ContinueWith(
            _ => {
                Console.WriteLine("continuation 2 called");
            },
            CancellationToken.None,
            TaskContinuationOptions.RunContinuationsAsynchronously,
            TaskScheduler.FromCurrentSynchronizationContext()
        );
        tcs1.SetResult();
    }
}

Exceptions (if any)

No response

.NET Version

6.0.302

Anything else?

ASP.NET Core version: 6.0.7

To resolve this issue I recommend to look closer at RendererSynchronizationContext. This context contains the _state.Task variable. This variable represents task chain, builded by this context. I think deadlock occurs when the chain closes itself. I attached screenshot to show structure of this variable.

RendererSyncContext State Task

The possible workaround to avoid this problem is specify TaskScheduler.Current instead of TaskScheduler.FromCurrentSynchronizationContext() in ContinueWith method.

javiercn commented 2 years ago

@JaleChaki thanks for contacting us.

We do not recommend using Task.ContinueWith directly from Blazor unless you really know what you are doing, as Blazor runs inside a synchronization context and the compiler adds additional machinery to do things correctly when using async/await (which is what we recommend).

We would recommend you try authoring the code with async/await instead of directly using ContinueWith and see if that addresses the problem.

ghost commented 2 years ago

Hi @JaleChaki. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

JaleChaki commented 2 years ago

Hello, I didn't found any restrictions on using ContinueWith from Blazor in the msdn docs. In my opinion ContinueWith is an important feature of asynchronous programming. I use it to schedule business logic from synchronous code: when component parameters change I make an asynchronous http request and schedule a continutation with custom logic when data is loaded. Of course I could write own task continuation wrapper, it will look like this:

async Task MyContinueWith(Task antecedentTask, CancellationToken token, Func<Task, Task> myContinuation) {
  try {
    await antecedentTask;
  }
  catch { }
  cancellationToken.ThrowIfCancellationRequested();
  await myContinuation(antecedentTask);
}

// usage from sync code
_ = MyContinueWith(dataLoadTask, token, onDataLoaded);

But this code is look like "bicycle" and makes any people wonder "why not use common continuation?". This approach doesn't allow you to specify how to run this logic (synchronously after task completion or asynchronously) and which TaskScheduler use for this.

Also I wrote that the problem occurs only on the SynchronizationContextTaskScheduler.

javiercn commented 2 years ago

@JaleChaki thanks for the additional details.

I did a bit more investigation and I am going to take a deeper look in the future, so I am going to include it in our planning process.

As for your comment regarding wrapping the code, the recommended way to do this would be to go async all the way. Your event handler can be Task returning and Blazor takes care of capturing cancellation exceptions, so you do not have to.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.