aspnet / Identity

[Archived] ASP.NET Core Identity is the membership system for building ASP.NET Core web applications, including membership, login, and user data. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.96k stars 869 forks source link

Running synchronous methods on UserManager means UserStore methods have no access to HttpContext.Current #1345

Closed ssteiner closed 7 years ago

ssteiner commented 7 years ago

I've built my own simple UserManager and SigninManager with a custom store storing accounts to a json file.

While testing, I came across something peculiar - in the end, the demonstrator serves as a template for a bigger endeavor where the IUserStore is based on a REST service, and to make remote calls, I need access to HttpContext.Current inside the IUserStore implementation.

Yet every so often, HttpContext.Current would be null. And I finally found the culprit: whenever I would be calling UserManager.DoSomething() from one of my controllers, the corresponding calls in my IUserStore implementation would have HttpContext.Current empty. If I instead called UserManager.DoSometingAsync(), HttpContext.Current would have a value.

It almost seems as if the sync methods simply wrap the async methods but run into a different context and in that different context, HttpContext.Current is not available.

To demonstrate this, I created a sample project. It's your standard MVC5 project where I just plugged in my own ASP.NET Identity store (all files are in the UserManagement folder). To see the problem, set three breakpoints in CustomUserStore.cs on lines 39 (CreateAsync method), 63 (FindByIdAsync) and 69 (FindByNameAsync) that all hit if ctx == null.

Run the project, and create a new account. You'll see breakpoints in FindByNameAsync and CreateAsync will actually break. This because I changed AccountController.Register to create the user synchronously. Revert back to asynchronous, and that problem goes away.

Another instance to see the problem is when you change a logged in user's password. Click on the link to manage your account, and the call to HasPassword() in ManageController.Index will trigger a breakpoint hit in CustomUserStore.FindByIdAsync. Replace HasPassword() with HasPasswordAsync() and the problem goes away.

So, now I'm left wondering whether there's an error in my implementation, or if the user manager does something iffy when running the synchronous methods. Given that IUserStore only demands asynchronous methods, I'd imagine that UserManager has some kind of logic to run the async methods in the IUserStore that makes the methods being executed on a different SynchronizationContext that does not have access to the usual ASP.NET properties (just like HttpContext.Current).

My sample project can be found here

HaoK commented 7 years ago

This repo is only for Identity on AspNetCore, you appear to be using the older versions of aspnet/identity in your sample, feel free to file this issue over at https://aspnetidentity.codeplex.com/workitem/list/basic

But in general yes, async can cause issues with HttpContext.Current if it runs on a different thread.

https://stackoverflow.com/questions/19111218/httpcontext-current-null-inside-async-task