Confused by TaskScheduler and SynchronizationContext for sync-on-async, can't control the sync context?
Asked Answered
T

1

7

The Issue

I have an ASP.NET 4.0 WebForms page with a simple web-service WebMethod. This method is serving as a synchronous wrapper to asynchronous / TPL code. The problem I'm facing, is that the inner Task sometimes has a null SynchronizationContext (my preference), but sometimes has a sync context of System.Web.LegacyAspNetSynchronizationContext. In the example I've provided, this doesn't really cause a problem, but in my real-world development scenario can lead to dead-locks.

The first call to the service always seems to run with null sync context, the next few might too. But a few rapid-fire requests and it starts popping onto the ASP.NET sync context.

The Code

[WebMethod]
public static string MyWebMethod(string name)
{
    var rnd = new Random();
    int eventId = rnd.Next();
    TaskHolder holder = new TaskHolder(eventId);

    System.Diagnostics.Debug.WriteLine("Event Id: {0}. Web method thread Id: {1}",
        eventId,
        Thread.CurrentThread.ManagedThreadId);

    var taskResult = Task.Factory.StartNew(
        function: () => holder.SampleTask().Result,
        creationOptions: TaskCreationOptions.None,
        cancellationToken: System.Threading.CancellationToken.None,
        scheduler: TaskScheduler.Default)
        .Result;

    return "Hello " + name + ", result is " + taskResult;
}

The definition of TaskHolder being:

public class TaskHolder
{
    private int _eventId;
    private ProgressMessageHandler _prg;
    private HttpClient _client;

    public TaskHolder(int eventId)
    {
        _eventId = eventId;
        _prg = new ProgressMessageHandler();
        _client = HttpClientFactory.Create(_prg);
    }

    public Task<string> SampleTask()
    {
        System.Diagnostics.Debug.WriteLine("Event Id: {0}. Pre-task thread Id: {1}",
            _eventId,
            Thread.CurrentThread.ManagedThreadId);

        return _client.GetAsync("http://www.google.com")
            .ContinueWith((t) =>
                {
                    System.Diagnostics.Debug.WriteLine("Event Id: {0}. Continuation-task thread Id: {1}",
                        _eventId,
                        Thread.CurrentThread.ManagedThreadId);

                    t.Wait();

                    return string.Format("Length is: {0}", t.Result.Content.Headers.ContentLength.HasValue ? t.Result.Content.Headers.ContentLength.Value.ToString() : "unknown");
                }, scheduler: TaskScheduler.Default);
    }
}

Analysis

My understanding of TaskScheduler.Default is that it's the ThreadPool scheduler. In other words, the thread won't end up on the ASP.NET thread. As per this article, "The default scheduler for Task Parallel Library and PLINQ uses the .NET Framework ThreadPool to queue and execute work". Based on that, I would expect the SynchronizationContext inside SampleTask to always be null.

Furthermore, my understanding is that if SampleTask were to be on the ASP.NET SynchronizationContext, the call to .Result in MyWebMethod may deadlock.

Because I'm not going "async all the way down", this is a "synchronous-on-asynchronous" scenario. Per this article by Stephen Toub, in the section titled "What if I really do need “sync over async”?" the following code should be a safe wrapper:

Task.Run(() => holder.SampleTask()).Result

According to this other article, also by Stephen Toub, the above should be functionally equivalent to:

Task.Factory.StartNew(
    () => holder.SampleTask().Result, 
    CancellationToken.None, 
    TaskCreationOptions.DenyChildAttach, 
    TaskScheduler.Default);

Thanks to being in .NET 4.0, I don't have access to TaskCreationOptions.DenyChildAttach, and I thought this was my issue. But I've run up the same sample in .NET 4.5 and switched to TaskCreationOptions.DenyChildAttach and it behaves the same (sometimes grabs the ASP.NET sync context).

I decided then to go closer to the "original" recommendation, and implement in .NET 4.5:

Task.Run(() => holder.SampleTask()).Result

And this does work, in that it always has a null sync context. Which, kind of suggests the Task.Run vs Task.Factory.StartNew article has it wrong?

The pragmatic approach would be to upgrade to .NET 4.5 and use the Task.Run implementation, but that would involve development time that I'd rather spend on more pressing issues (if possible). Plus, I'd still like to figure out what's going on with the different TaskScheduler and TaskCreationOptions scenarios.

I've coincidentally found that TaskCreationOptions.PreferFairness in .NET 4.0 appears to behave as I'd wish (all executions have a null sync context), but without knowing why this works, I'm very hesitant to use it (it may not work in all scenarios).

Edit

Some extra info... I've updated my sample code with one that does deadlock, and includes some debug output to show what threads the tasks are running on. A deadlock will occur if either the pre-task or continuation-task outputs indicate the same thread id as the WebMethod.

Curiously, if I don't use ProgressMessageHandler, I don't seem able to replicate the deadlock. My impression was that this shouldn't matter, that regardless of down-stream code, I should be able to safely "wrap" an asynchronous method up in a synchronous context using the right Task.Factory.StartNew or Task.Run method. But this doesn't seem to be the case?

Tinnitus answered 20/2, 2013 at 6:11 Comment(4)
Could you explain why do you need sync-over-async? And why are you event using async here, instead of running the code directly? Because it seems async doesn't give you any benefit.Henkel
I'm forced into this scenario by wanting (needing?) to use the HttpClient class to communicate with an ASP.NET Web API implementation. HttpClient is entirely async, whether I like it or not.Tinnitus
What I meant is why can't you go async all the way? That forces you to use sync?Henkel
I am working in a large legacy code-base that cannot reasonably be migrated to an "async all the way down" pattern. If this code were for a new application, I would go async all the way. I'd also be using 4.5 instead of 4.0.Tinnitus
H
4

First, using sync-over-async in ASP.NET often doesn't make much sense. You're incurring the overhead of creating and scheduling Tasks, but you don't benefit from it in any way.

Now, to your question:

My understanding of TaskScheduler.Default is that it's the ThreadPool scheduler. In other words, the thread won't end up on the ASP.NET thread.

Well, ASP.NET uses the same ThreadPool too. But that's not really relevant here. What's relevant is that if you Wait() (or call Result, that's the same) on a Task that's scheduled to run (but didn't start yet), the TaskScheduler my decide to just run your Task synchronously. This is known as “task inlining”.

What this means is that your Task ends up running on the SynchronizationContext, but it wasn't actually scheduled through it. This means there is actually no risk of deadlocks.

Thanks to being in .NET 4.0, I don't have access to TaskCreationOptions.DenyChildAttach, and I thought this was my issue.

This has nothing to do with DenyChildAttach, there are no Tasks that would be AttachedToParent.

I've coincidentally found that TaskCreationOptions.PreferFairness in .NET 4.0 appears to behave as I'd wish (all executions have a null sync context), but without knowing why this works, I'm very hesitant to use it (it may not work in all scenarios).

This is because PreferFairness schedules the Task to the global queue (instead of the thread-local queue that each ThreadPool thread has), and it seems Tasks from the global queue won't be inlined. But I wouldn't rely on this behavior, especially since it can change in the future.

EDIT:

Curiously, if I don't use ProgressMessageHandler, I don't seem able to replicate the deadlock.

There's nothing curious about that, that's exactly your problem. ProgressMessageHandler reports progress on the current synchronization context. And because of task inlining, that is the ASP.NET context, which you're blocking by waiting synchronously.

What you need to do is to make sure GetAsync() is run on a thread without the synchronization context set. I think the best way to do that is to call SynchronizationContext.SetSynchronizationContext(null) before calling GetAsync() and restoring it afterwards.

Henkel answered 20/2, 2013 at 11:0 Comment(11)
"What this means is that your Task ends up running on the SynchronizationContext" Wouldn't this mean that the task's body runs under an unexpected syncctx and if the body itself starts and waits on tasks it might deadlock now?Factual
@Factual No, because if you start a Task (without specifying a scheduler), it will run using the current scheduler, but not using the current synchronization context. It would be different if the question was about using await, but it isn't.Henkel
Thanks for the response svick, I've updated my question with code that does deadlock whenever the Task(s) get inlined. I appreciate that sync-over-async is a wacky scenario, but I'm trying to make use of HttpClient which is entirely async and am working in a large legacy code-base that cannot reasonably be migrated to an "async all the way down" pattern.Tinnitus
@Henkel I was just coming to the same conclusion myself having looked at the implementation of ProgressMessageHandler.AddResponseProgress and its usage of TaskHelpersExtensions.ThenImplContinuation. With ThenImplContinuation grabbing the current sync context and posting to it. The block is actually on the sync context Post method call (posting in this case to the AspNet sync context), which surprises me, because I thought Post was non-blocking. I guess the problem is execution of the posted callback is non-blocking for the poseter, but adding it to the queue isn't?Tinnitus
@Tinnitus The problem is, the code that's posted to the synchronization context sets the Task as completed. So, Post() is non-blocking, the method returns and you start waiting on a Task. But that Task never completes, because the Post()ed delegate never executes.Henkel
In ASP.NET I could just dodge usage of ProgressMessageHandler. It's only there because I'm putting together a "client" library which uses it by default. In my WinForm apps this isn't a problem because the WinForm sync context doesn't run on ThreadPool (unlike ASP.NET), so will never in-line a task. I'm left wondering now why I couldn't get this to deadlock when using Task.Run though. Does it somehow have different in-lining rules?Tinnitus
@Henkel The call in TaskHelpersExtensions.ThenImplContinuation to SynchronizationContext.Post is definitely blocking. I'm running a debug build of the WebStack and it's clearly showing multiple threads blocked in ThenImplContinuation on the call to Post.Tinnitus
@Tinnitus You're right, Post() blocks in ASP.NET, see this article.Henkel
I'm starting to think the only way I can guarantee a safe synchronous wrapper around an async method is to use TaskScheduler.Default and TaskCreationOptions.LongRunning to force the Task off the ThreadPool. I could also manually clear the SynchronizationContext as you've suggested, but I'm left wondering what other traps there are. I thought, based on blogs and articles, that TaskScheduler.Default alone was sufficient, and in most cases... it was.Tinnitus
Oh wow - "When a delegate is queued to a captured AspNetSynchronizationContext, it restores the identity and culture of the original page and then executes the delegate directly. The delegate is directly invoked even if it’s “asynchronously” queued by calling Post" - Everything has a caveat in async doesn't it?Tinnitus
let us continue this discussion in chatTinnitus

© 2022 - 2024 — McMap. All rights reserved.