Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

AspNetSynchronizationContext and await continuations in ASP.NET

I noticed an unexpected (and I'd say, a redundant) thread switch after await inside asynchronous ASP.NET Web API controller method.

For example, below I'd expect to see the same ManagedThreadId at locations #2 and 3#, but most often I see a different thread at #3:

public class TestController : ApiController
{
    public async Task<string> GetData()
    {
        Debug.WriteLine(new
        {
            where = "1) before await",
            thread = Thread.CurrentThread.ManagedThreadId,
            context = SynchronizationContext.Current
        });

        await Task.Delay(100).ContinueWith(t =>
        {
            Debug.WriteLine(new
            {
                where = "2) inside ContinueWith",
                thread = Thread.CurrentThread.ManagedThreadId,
                context = SynchronizationContext.Current
            });
        }, TaskContinuationOptions.ExecuteSynchronously); //.ConfigureAwait(false);

        Debug.WriteLine(new
        {
            where = "3) after await",
            thread = Thread.CurrentThread.ManagedThreadId,
            context = SynchronizationContext.Current
        });

        return "OK";
    }
}

I've looked at the implementation of AspNetSynchronizationContext.Post, essentially it comes down to this:

Task newTask = _lastScheduledTask.ContinueWith(_ => SafeWrapCallback(action));
_lastScheduledTask = newTask;

Thus, the continuation is scheduled on ThreadPool, rather than gets inlined. Here, ContinueWith uses TaskScheduler.Current, which in my experience is always an instance of ThreadPoolTaskScheduler inside ASP.NET (but it doesn't have to be that, see below).

I could eliminate a redundant thread switch like this with ConfigureAwait(false) or a custom awaiter, but that would take away the automatic flow of the HTTP request's state properties like HttpContext.Current.

There's another side effect of the current implementation of AspNetSynchronizationContext.Post. It results in a deadlock in the following case:

await Task.Factory.StartNew(
    async () =>
    {
        return await Task.Factory.StartNew(
            () => Type.Missing,
            CancellationToken.None,
            TaskCreationOptions.None,
            scheduler: TaskScheduler.FromCurrentSynchronizationContext());
    },
    CancellationToken.None,
    TaskCreationOptions.None,
    scheduler: TaskScheduler.FromCurrentSynchronizationContext()).Unwrap();

This example, albeit a bit contrived, shows what may happen if TaskScheduler.Current is TaskScheduler.FromCurrentSynchronizationContext(), i.e., made from AspNetSynchronizationContext. It doesn't use any blocking code and would have been executed smoothly in WinForms or WPF.

This behavior of AspNetSynchronizationContext is different from the v4.0 implementation (which is still there as LegacyAspNetSynchronizationContext).

So, what is the reason for such change? I thought, the idea behind this might be to reduce the gap for deadlocks, but deadlock are still possible with the current implementation, when using Task.Wait() or Task.Result.

IMO, it'd more appropriate to put it like this:

Task newTask = _lastScheduledTask.ContinueWith(_ => SafeWrapCallback(action),
    TaskContinuationOptions.ExecuteSynchronously);
_lastScheduledTask = newTask;

Or, at least, I'd expect it to use TaskScheduler.Default rather than TaskScheduler.Current.

If I enable LegacyAspNetSynchronizationContext with <add key="aspnet:UseTaskFriendlySynchronizationContext" value="false" /> in web.config, it works as desired: the synchronization context gets installed on the thread where the awaited task has ended, and the continuation is synchronously executed there.

like image 547
noseratio Avatar asked Apr 14 '14 13:04

noseratio


2 Answers

That the continuation is being dispatched onto a new thread rather than inlined is intentional. Let's break this down:

  1. You're calling Task.Delay(100). After 100 milliseconds, the underlying Task will transition to a completed state. But that transition will happen on an arbitrary ThreadPool / IOCP thread; it won't happen on a thread under the ASP.NET sync context.

  2. The .ContinueWith(..., ExecuteSynchronously) will cause the Debug.WriteLine(2) to take place on the thread that transitioned the Task.Delay(100) to a terminal state. ContinueWith will itself return a new Task.

  3. You're awaiting the Task returned by [2]. Since the thread which completes Task [2] isn't under the control of the ASP.NET sync context, the async / await machinery will call SynchronizationContext.Post. This method is contracted always to dispatch asynchronously.

The async / await machinery does have some optimizations to execute continuations inline on the completing thread rather than calling SynchronizationContext.Post, but that optimization only kicks in if the completing thread is currently running under the sync context that it's about to dispatch to. This isn't the case in your sample above, as [2] is running on an arbitrary thread pool thread, but it needs to dispatch back to the AspNetSynchronizationContext to run the [3] continuation. This also explains why the thread hop doesn't occur if you use .ConfigureAwait(false): the [3] continuation can be inlined in [2] since it's going to be dispatched under the default sync context.

To your other questions re: Task.Wait() and Task.Result, the new sync context was not intended to reduce deadlock conditions relative to .NET 4.0. (In fact, it's slightly easier to get deadlocks in the new sync context than it was in the old context.) The new sync context was intended to have an implementation of .Post() that plays well with the async / await machinery, which the old sync context failed miserably at doing. (The old sync context's implementation of .Post() was to block the calling thread until the synchronization primitive was available, then dispatch the callback inline.)

Calling Task.Wait() and Task.Result from the request thread on a Task not known to be completed can still cause deadlocks, just like calling Task.Wait() or Task.Result from the UI thread in a Win Forms or WPF application.

Finally, the weirdness with Task.Factory.StartNew might be an actual bug. But until there's an actual (non-contrived) scenario to support this, the team would not be inclined to investigate this further.

like image 86
Levi Avatar answered Oct 15 '22 13:10

Levi


Now my guess is, they have implemented AspNetSynchronizationContext.Post this way to avoid a possibility of infinite recursion which might lead to stack overflow. That might happen if Post is called from the callback passed to Post itself.

Still, I think an extra thread switch might be too expensive for this. It could have been possibly avoided like this:

var sameStackFrame = true
try
{
    //TODO: also use TaskScheduler.Default rather than TaskScheduler.Current 
    Task newTask = _lastScheduledTask.ContinueWith(completedTask => 
    {
        if (sameStackFrame) // avoid potential recursion
           return completedTask.ContinueWith(_ => SafeWrapCallback(action));
        else 
        {
           SafeWrapCallback(action);
           return completedTask;
        }
    }, TaskContinuationOptions.ExecuteSynchronously).Unwrap();

    _lastScheduledTask = newTask;    
}
finally
{
    sameStackFrame = false;
}

Based on this idea, I've created a custom awaiter which gives me the desired behavior:

await task.ConfigureContinue(synchronously: true);

It uses SynchronizationContext.Post if operation completed synchronously on the same stack frame, and SynchronizationContext.Send if it did on a different stack frame (it could even be the same thread, asynchronously reused by ThreadPool after some cycles):

using System;
using System.Diagnostics;
using System.Runtime.Remoting.Messaging;
using System.Threading;
using System.Threading.Tasks;
using System.Web;
using System.Web.Http;

namespace TestApp.Controllers
{
    /// <summary>
    /// TestController
    /// </summary>
    public class TestController : ApiController
    {
        public async Task<string> GetData()
        {
            Debug.WriteLine(String.Empty);

            Debug.WriteLine(new
            {
                where = "before await",
                thread = Thread.CurrentThread.ManagedThreadId,
                context = SynchronizationContext.Current
            });

            // add some state to flow
            HttpContext.Current.Items.Add("_context_key", "_contextValue");
            CallContext.LogicalSetData("_key", "_value");

            var task = Task.Delay(100).ContinueWith(t =>
            {
                Debug.WriteLine(new
                {
                    where = "inside ContinueWith",
                    thread = Thread.CurrentThread.ManagedThreadId,
                    context = SynchronizationContext.Current
                });
                // return something as we only have the generic awaiter so far
                return Type.Missing; 
            }, TaskContinuationOptions.ExecuteSynchronously);

            await task.ConfigureContinue(synchronously: true);

            Debug.WriteLine(new
            {
                logicalData = CallContext.LogicalGetData("_key"),
                contextData = HttpContext.Current.Items["_context_key"],
                where = "after await",
                thread = Thread.CurrentThread.ManagedThreadId,
                context = SynchronizationContext.Current
            });

            return "OK";
        }
    }

    /// <summary>
    /// TaskExt
    /// </summary>
    public static class TaskExt
    {
        /// <summary>
        /// ConfigureContinue - http://stackoverflow.com/q/23062154/1768303
        /// </summary>
        public static ContextAwaiter<TResult> ConfigureContinue<TResult>(this Task<TResult> @this, bool synchronously = true)
        {
            return new ContextAwaiter<TResult>(@this, synchronously);
        }

        /// <summary>
        /// ContextAwaiter
        /// TODO: non-generic version 
        /// </summary>
        public class ContextAwaiter<TResult> :
            System.Runtime.CompilerServices.ICriticalNotifyCompletion
        {
            readonly bool _synchronously;
            readonly Task<TResult> _task;

            public ContextAwaiter(Task<TResult> task, bool synchronously)
            {
                _task = task;
                _synchronously = synchronously;
            }

            // awaiter methods
            public ContextAwaiter<TResult> GetAwaiter()
            {
                return this;
            }

            public bool IsCompleted
            {
                get { return _task.IsCompleted; }
            }

            public TResult GetResult()
            {
                return _task.Result;
            }

            // ICriticalNotifyCompletion
            public void OnCompleted(Action continuation)
            {
                UnsafeOnCompleted(continuation);
            }

            // Why UnsafeOnCompleted? http://blogs.msdn.com/b/pfxteam/archive/2012/02/29/10274035.aspx
            public void UnsafeOnCompleted(Action continuation)
            {
                var syncContext = SynchronizationContext.Current;
                var sameStackFrame = true; 
                try
                {
                    _task.ContinueWith(_ => 
                    {
                        if (null != syncContext)
                        {
                            // async if the same stack frame
                            if (sameStackFrame)
                                syncContext.Post(__ => continuation(), null);
                            else
                                syncContext.Send(__ => continuation(), null);
                        }
                        else
                        {
                            continuation();
                        }
                    }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default);
                }
                finally
                {
                    sameStackFrame = false;
                }
            }
        }
    }
}
like image 25
noseratio Avatar answered Oct 15 '22 12:10

noseratio