3

As you cannot run async methods from child window (@Html.Action) calls, I have been searching for the simplest way to run async tasks from a non-async method. This will allow my MainMenu controller Menu action to still work when injected like this (rather than have to move to a VM or Ajax solution):

<div class="row">
    @Html.Action("MainMenu", "Menu")
</div>

I tried this promising approach using a copy of the code MS use themselves: How to call asynchronous method from synchronous method in C#?

AsyncHelper code:

public static class AsyncHelper
{
    private static readonly TaskFactory _myTaskFactory = new
          TaskFactory(CancellationToken.None,
                      TaskCreationOptions.None,
                      TaskContinuationOptions.None,
                      TaskScheduler.Default);

    public static TResult RunSync<TResult>(Func<Task<TResult>> func)
    {
        return AsyncHelper._myTaskFactory
          .StartNew<Task<TResult>>(func)
          .Unwrap<TResult>()
          .GetAwaiter()
          .GetResult();
    }

    public static void RunSync(Func<Task> func)
    {
        AsyncHelper._myTaskFactory
          .StartNew<Task>(func)
          .Unwrap()
          .GetAwaiter()
          .GetResult();
    }
}

and I am consuming it like this:

    public async Task<ActionResult> MainMenu()
    {
        if (_currentCandidate == null)
        {
            throw new ArgumentNullException("_currentCandidate");
        }
        var candidateId = AsyncHelper.RunSync<int>(() => _currentCandidate.CandidateIdAsync());

        [snip]
    }

Which calls this async method:

    public async Task<int> CandidateIdAsync()
    {
        var applicationUser = await this.ApplicationUserAsync();
        if (applicationUser != null)
        {
            return applicationUser.CandidateId.GetValueOrDefault();
        }
        return 0;
    }

Only when I run this I get the following error:

enter image description here

What am I missing here? The code looks like it should work, but I am not familiar enough with it yet to figure it out.

Update:

For reference the MainMenu controller class looks like this:

public class MenuController : Controller
{
    readonly ICurrentCandidate _currentCandidate;

    public MenuController(ICurrentCandidate currentCandidate)
    {
        _currentCandidate = currentCandidate;
    }

    // GET: MainMenu
    public ActionResult MainMenu()
    {
        if (_currentCandidate == null)
        {
            throw new ArgumentNullException("_currentCandidate");
        }
        var candidateId = AsyncHelper.RunSync<int>(() => _currentCandidate.CandidateIdAsync());

        [snip]            
        return View(vm);
    }
}

Another update:

The failure appears to be inside the related IF code as a simplified CandidateIdAsnyc works:

// This works
public async Task<int> CandidateIdAsync()
{
    return 0;
}

Here is the rest of that code:

public class CurrentCandidate : ICurrentCandidate
{
    private readonly ApplicationDbContext _applicationDbContext;
    private readonly IApplicationUserManager _userManager;
    private readonly ICandidateStore _candidateStore;

    public CurrentCandidate(ApplicationDbContext applicationDbContext, ICandidateStore candidateStore, IApplicationUserManager userManager)
    {
        this._candidateStore = candidateStore;
        this._applicationDbContext = applicationDbContext;
        this._userManager = userManager;    // new ApplicationUserManager(new UserStore<ApplicationUser>(this._applicationDbContext));
    }

    public async Task<ApplicationUser> ApplicationUserAsync()
    {
        var applicationUser = await this._userManager.FindByIdAsync(HttpContext.Current.User.Identity.GetUserId());
        return applicationUser;
    }

    public bool IsAuthenticated()
    {
        return HttpContext.Current.User.Identity.IsAuthenticated;
    }

    public async Task<int> CandidateIdAsync()
    {
        var applicationUser = await this.ApplicationUserAsync();
        if (applicationUser != null)
        {
            return applicationUser.CandidateId.GetValueOrDefault();
        }
        return 0;
    }
}
Community
  • 1
  • 1
iCollect.it Ltd
  • 92,391
  • 25
  • 181
  • 202
  • 1
    You need to make a piece of sample code that actually replicates your problem. I've tried something as similar as I could get, and it works with no problems. It might also be in your `ApplicationUserAsync` method. Whittle it down to the simplest piece of code that actually displays the improper behaviour. – Luaan Feb 23 '15 at 17:22
  • @Luaan: That is all standard identity framework components, so I will replace it with something simpler that just returns the 0. – iCollect.it Ltd Feb 23 '15 at 17:24
  • Why not basic `var r = CandidateIdAsync().ConfigureAwait(false).Result` ? – Alexei Levenkov Feb 23 '15 at 17:25
  • Also, why are you using `Action` for this? Wouldn't `Partial` work better? I'm not sure if this is a proper use of the MVC pattern... – Luaan Feb 23 '15 at 17:25
  • @Luaan: I can't use a partial as the menu is injected into layouts as well as views. You were onto something though as a simplified `CandidateIdAsync` works – iCollect.it Ltd Feb 23 '15 at 17:26
  • 1
    Note that most in methods in System.Web and (similar Web-related assemblies) expect HttpContext.Current and Culture to be set - most ways that show how to run async mehtod synchronously in ASP.Net will run method on thread where `HttpContext.Current` is not set (including `ConfigureAwait` I suggested). – Alexei Levenkov Feb 23 '15 at 17:27
  • @AlexeiLevenkov Letting the async action complete on the thread pool prevents you from a lot of ugly deadlock issues if you need to synchronize back on the original context. It's very easy to deadlock on `Result`. – Luaan Feb 23 '15 at 17:27
  • @Alexei Levenkov: `ConfigureAwait(false)` does not have a `Result` property – iCollect.it Ltd Feb 23 '15 at 17:27
  • 1
    That's the problem - `RunSync` doesn't synchronize the async methods back to the request context. So your code is failing because it depends on something in the current request's context - which in MVC means pretty much everything. You can't use it like that - you need to pass everything from the outside. In other words, you're in trouble :D So without changing your architecture a lot, your only way is to use `Result` on the task directly - and that's a great way to get tons of deadlocks when your code gets a bit more complicated. – Luaan Feb 23 '15 at 17:28
  • @TrueBlueAussie - you could convince it to compile by splitting into 2 statements, but it is not going to help you due to my other comment... – Alexei Levenkov Feb 23 '15 at 17:29
  • @Luaan: The underlying components are all IdentityFramework. I will show more of the calling code that is failing for you. – iCollect.it Ltd Feb 23 '15 at 17:31
  • Yeah, IdentityFramework definitely needs the context - it takes the `Identity` out of it :) – Luaan Feb 23 '15 at 17:31
  • @Luaan - it is possible to work around so - one can set HttpContext by hand if code needs context only in sync portion and even create custom Synchronization Context that will not deadlock (now properly serializing access to actual current `HttpContext` object is tricky :) ) – Alexei Levenkov Feb 23 '15 at 17:32

3 Answers3

4

I have been searching for the simplest way to run async tasks from a non-async method.

This has been discussed many times, and there is no solution that works in every scenario. The internal AsyncHelper type is used only in very specific situations where the ASP.NET team knows it's safe; it's not a general-purpose solution.

Generally, the approaches are:

  1. Block (using Result or GetAwaiter().GetResult()). This approach can cause deadlocks (as I describe on my blog) unless you consistently use ConfigureAwait(false) - and all of the code you call also consistently uses ConfigureAwait(false). However, note that your code can't use ConfigureAwait(false) unless it doesn't actually need the ASP.NET context.
  2. Nested message loop. This is possible using something like AsyncContext from my AsyncEx library. However, there are a lot of ASP.NET APIs that implicitly assume the current SynchronizationContext is AspNetSynchronizationContext, which is not the case within AsyncContext.
  3. Separate thread with blocking (using Task.Run(...).GetAwaiter().GetResult()). This approach avoids the deadlocks you can see with just blocking, but it does execute the code outside of an ASP.NET context. This approach also negatively impacts your scalability (which is the whole point of using async on ASP.NET in the first place).

In other words, these are just hacks.

ASP.NET vNext has the concept of "view components" which may be async, so this will be a natural solution in the future. For today's code, IMO the best solution is to make the methods synchronous; that's better than implementing a hack.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
1

Thanks for the advice everyone.

None of the "workarounds" worked, and we needed to keep existing async code, so I decided not to fight against async.

I have avoided the problem by not using @Html.Action at all.

Instead I use the menu as a partial view using

@Html.Partial("MainMenu", @ViewBag.MainMenuViewModel);

and setting that model in our base controller.

iCollect.it Ltd
  • 92,391
  • 25
  • 181
  • 202
  • Short of going to ASP.Net VNext (aka ASP.Net MVC6) which supports asynchronous calls from view (via [components](https://www.asp.net/vnext/overview/aspnet-vnext/vc)) moving asynchronous calls to main action is my preferred option. You don't lose any [culture](http://stackoverflow.com/questions/28117913/culture-not-being-applied-in-multilingual-site/28247735)/`HttpContex` this way and can use regular `async`/`await` without resorting to synchronous waits (which negate all benefits of `async`). – Alexei Levenkov Feb 24 '15 at 15:35
-1

Quite standard way of running async methods synchronously is

var r = Task.Run( () => MyAsynchMethod(args)).Result;

Or

var task = MyAsynchMethod(args);
task.ConfigureAwait(false);
var r = task.Result;

Both will not cause deadlock as they will run at least asynchronous return part of the method without ASP.Net Synchronization context.

Now most ASP.Net-related methods (like rendering or identity in your case) expect HttpContext.Current to be correctly set to "current" context - which is explicitly not the case when code ends up running on thread that did not "entered" ASP.Net synchronization context (you also loose current culture, but at least it will not cause NRE).

Dirty workaround - set context manually, but you run into danger of accessing same context in parallel from multiple threads - if used carefully it may be ok (i.e. if "main" request thread is waiting on .Result other thread can use it):

 var context = HttpContext.Current;  
 var r = Task.Run( () => 
    {
       HttpContext.Current = context;
       return MyAsynchMethod(args);
    }
).Result;

Note that you'd better restore context and set/restore both CurrentCulture/CurrentUICulture too.

Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179
  • 1
    With the first examples, I get `"An exception of type 'System.AggregateException' occurred in mscorlib.dll but was not handled in user code"` – iCollect.it Ltd Feb 24 '15 at 10:12
  • and I can't get the second example to compile. Where is `Current` defined? – iCollect.it Ltd Feb 24 '15 at 10:14
  • @TrueBlueAussie- Exceptions are expected because they would be the same NRE you see already, just wrapped in AggregateException since there is not `HttpContext.Current`( which is `System.Web.HttpContext.Current` from System.Web assembly). – Alexei Levenkov Feb 24 '15 at 15:28
  • I get the same exception when I 'Windows.Storage.FileIO.ReadTextAsync'. So have can I use this Task.Run( () => ... to work? –  Apr 16 '15 at 13:36