Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introducing LightInject Produces Scoping Error #54

Closed
Mike-E-angelo opened this issue Feb 4, 2020 · 14 comments
Closed

Introducing LightInject Produces Scoping Error #54

Mike-E-angelo opened this issue Feb 4, 2020 · 14 comments

Comments

@Mike-E-angelo
Copy link

Mike-E-angelo commented Feb 4, 2020

Apologies in advance as this appears to be a complicated issue, and my limited knowledge of the new ASP.NET Core framework at the moment is keeping me from simplifying this further.

I have been fighting for the past day with this issue, and have reduced it to the following factors:

  • ASP.NET Core Blazor Server-Side
  • Twitter Authentication (unfortunately this doesn't reproduce with built-in authentication and as mentioned I have not seen another, simpler way to reproduce this)
  • Using LightInject as the DI

The problem appears to be a scoping issue. The RemoteNavigationManager gets created twice on the same request, even though it's scoped to not do so.

When authenticating, the request returns from Twitter and sends the user back to the root URL, where this error then occurs. Here is the stack trace:

InvalidOperationException: 'RemoteNavigationManager' has not been initialized.
Microsoft.AspNetCore.Components.NavigationManager.AssertInitialized()
Microsoft.AspNetCore.Components.NavigationManager.remove_LocationChanged(EventHandler<LocationChangedEventArgs> value)
Microsoft.AspNetCore.Components.Routing.NavLink.Dispose()
Microsoft.AspNetCore.Components.Rendering.ComponentState.Dispose()
Microsoft.AspNetCore.Components.RenderTree.Renderer.Dispose(bool disposing)
Microsoft.AspNetCore.Components.Rendering.HtmlRenderer.HandleException(Exception exception)
Microsoft.AspNetCore.Components.RenderTree.Renderer.Dispose(bool disposing)
Microsoft.AspNetCore.Components.RenderTree.Renderer.Dispose()
Microsoft.AspNetCore.Mvc.ViewFeatures.StaticComponentRenderer.PrerenderComponentAsync(ParameterView parameters, HttpContext httpContext, Type componentType)
Microsoft.AspNetCore.Mvc.ViewFeatures.ComponentRenderer.PrerenderedServerComponentAsync(HttpContext context, ServerComponentInvocationSequence invocationId, Type type, ParameterView parametersCollection)
Microsoft.AspNetCore.Mvc.ViewFeatures.ComponentRenderer.RenderComponentAsync(ViewContext viewContext, Type componentType, RenderMode renderMode, object parameters)
Microsoft.AspNetCore.Mvc.TagHelpers.ComponentTagHelper.ProcessAsync(TagHelperContext context, TagHelperOutput output)
Microsoft.AspNetCore.Razor.Runtime.TagHelpers.TagHelperRunner.<RunAsync>g__Awaited|0_0(Task task, TagHelperExecutionContext executionContext, int i, int count)
Scopes.Pages.Pages__Host.<ExecuteAsync>b__14_1() in _Host.cshtml
+
        <component type="typeof(App)" render-mode="ServerPrerendered" />
Microsoft.AspNetCore.Razor.Runtime.TagHelpers.TagHelperExecutionContext.SetOutputContentAsync()
Scopes.Pages.Pages__Host.ExecuteAsync() in _Host.cshtml
+
    Layout = null;
Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageCoreAsync(IRazorPage page, ViewContext context)
Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageAsync(IRazorPage page, ViewContext context, bool invokeViewStarts)
Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderAsync(ViewContext context)
Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor.ExecuteAsync(ViewContext viewContext, string contentType, Nullable<int> statusCode)
Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor.ExecuteAsync(ViewContext viewContext, string contentType, Nullable<int> statusCode)
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|29_0<TFilter, TFilterAsync>(ResourceInvoker invoker, Task lastTask, State next, Scope scope, object state, bool isCompleted)
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext<TFilter, TFilterAsync>(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeResultFilters()
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResourceFilter>g__Awaited|24_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, object state, bool isCompleted)
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()
Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.MigrationsEndPointMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware.Invoke(HttpContext httpContext)
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware.Invoke(HttpContext httpContext)
Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

I've stuck a breakpoint in the constructor of RemoteNavigationManager and see that it does get called twice on the same request, with a call from LightInject.Scope.GetScopedInstance being the parent call site.

So, it would seem that something strange is going on here.

FWIW I have tried the following DI containers and did not encounter this error with them:

  • AutoFac
  • Grace
  • Unity

So this appears to be, uh, scoped (sorry 😆) to LightInject specifically.

I have created a repo that showcases the issue here:
https://github.com/Mike-EEE/LightInject.Scopes

Here is a video that shows how to reproduce it:
https://youtu.be/do32eM4hvSk

Apologies again for the complexity of the issue. If I find away to simplify it I will be sure to do so and update here.

Until this issue is fixed, I am setting the render-mode value from ServerPrerendered to Server on this tag here. There appears to be some magic that occurs here that sidesteps this issue.

Please let me know if there is any further information I can provide to assist in diagnosing this issue.

@seesharper
Copy link
Owner

Hi Mike. Thanks for a very detailed report. Even a video. That is a first 😀

Is this something I can just run or do I need a database and possibly some twitter key/secret?

@Mike-E-angelo
Copy link
Author

Yeah, that's the tricky part about this and a large part on why I was hesitant to report it. It occurs during external login, which is a bit of an effort to setup correctly.

Fortunately, the documentation is pretty good:
https://docs.microsoft.com/en-us/aspnet/core/security/authentication/social/twitter-logins?view=aspnetcore-3.1

You'll have to create a Twitter app and get the key/secret pair to use that SLN. Once you do, right-click the project file and select Manage User Secrets:

And use this as a template:

{
  "TwitterApplicationSettings": {
    "Secret": "",
    "Key": ""
  }
}

The one gotcha that kept getting me during setting up the Twitter application is the callback URL. I kept getting 403 response errors after signing in because the callback URLs were not defined correctly.

For reference, here is what mine looks like, set to the values of the launchSettings.json file in that repo:

For your copy/pasting the values are:

https://localhost:44367/signin-twitter
https://localhost:57099/signin-twitter

BTW, if you happen to know another external login (Microsoft Account, Google) I am betting either one of those would work, too. It seems this is happening after successful external authentication so any external provider should do. Don't quote me on that though, I am trying to save you some grief -- any grief at all! 😅

@Mike-E-angelo
Copy link
Author

Oh yeah... as for EntityFramework, that's actually all very nice these days. After logging in you will get a nice UI letting you know the database needs to be created with migrations applied:

And after applying it, it will prompt you to refresh:

Kinda cool. 😁

@seesharper
Copy link
Owner

Hi again Mike. I'll do my best to try to recreate this. I'm usually on a Mac so I need to update my Windows VM to debug this. I'll keep you posted 👍😀

@Mike-E-angelo
Copy link
Author

Wow, you develop on a Mac?! That makes LightInject that much more impressive. 😆 (Bad) jokes aside please let me know of any issues you encounter and I'll try my best to help you through them.

@seesharper
Copy link
Owner

Hi again. I've managed to reproduce the issue and just started to look into what is going on here.
I do agree that this looks like a scoping issue, but so far I have not been able to figure out where and when things starts to break.
What I have done so far is that I have included all the source files from LightInject, LightInject.Microsoft.DependencyInjection and LightInject.Microsoft.Hosting into the project so that it will be easier to debug.

The fork is here https://github.com/seesharper/LightInject.Scopes

I would really appreciate a second set of eyes on this as I could not immediately see anything fishy apart from the fact that the RemoteNavigationManager seems to be created multiple times. There might be reasons fro that since we are probably dealing with multiple requests here, because of the callback and all. Would really appreciate if you could take a look also and see if there is anything that sets of an alarm in your brain:)

@Mike-E-angelo
Copy link
Author

COOL you are able to reproduce it. I was certain that for sure that this was going to boil down to something crazy with my local environment. :)

As for assisting... I am unfortunately not too familiar with how scoping works with ASP.NET core -- or really, anything ASP.NET Core 😅 -- so it will take some time to ramp up. Have you been able to add Grace/AutoFac/Unity and see what the difference is when using those DI containers, by chance? All of them manage to load without issue.

That stated, I will take a look into the fork and see if there is anything obvious that sticks out. Thank you for creating it (and for looking into this!). 👍

@seesharper
Copy link
Owner

A little bit of update on this issue. I have been debugging the hell out this thing and I think we have a race condition related to how instances are cached in a given scope. Anyway, I just wanted to let you know that I have not given up on this👍

@Mike-E-angelo
Copy link
Author

woohoo! That's great news! I actually have this on my // TODO list and was going to attend to it once I finished the story I am working on now. My other thought was that the scope could be getting disposed of improperly somehow but it sounds like that is not the case. Glad to hear you made progress!

@seesharper
Copy link
Owner

seesharper commented Feb 10, 2020

I think I finally got it. It turned out to be an AVL tree rotation bug where we mixed up the left nodes in a double rotation. I don't know if you are familiar with AVL trees, but the rotation is necessary to keep the the tree balanced. We use immutable AVL trees where lookup performance is crucial. I'm going to need a day or two to wrap this up, but it looks promising. And a big thanks goes out to you that reported this bug in such detail. 🎂Without this issue, chances would have been high that this sneaky bug could have lived a lot longer and caused some serious trouble. What happened here in this case it that we got back an uninitialized RemoteNavigationManager where we should have gotten an existing instance that actually was initialized. The AVL tree bug caused the cache to miss and bring back a new instance instead.

@Mike-E-angelo
Copy link
Author

Dude that is great news and is wonderful to hear, @seesharper! I thought for sure that this was a lost cause and was so edge case complicated that it would never be located, let alone solved. Obviously consider the particular set of cases to produce this -- and I even thought it was only on my machine. 😆

Much appreciation and respect for finding this and figuring it out!!!

@seesharper
Copy link
Owner

Hi again Mike. A week of debugging is hopefully over 😀

Could you try to bump

<PackageReference Include="LightInject.Microsoft.Hosting" Version="1.1.1" />

to

<PackageReference Include="LightInject.Microsoft.Hosting" Version="1.1.2" />

@Mike-E-angelo
Copy link
Author

IT WORKS... dude NICE JOB and thank you sooooo much for putting the effort in fixing this, including going through the hassle of creating a Twitter app! That's COMMENDABLE. I am going to post about your achievement on Twitter now. :)

@Mike-E-angelo
Copy link
Author

Boom.

https://twitter.com/MikeTripleE/status/1227622981654007808

Closing this issue now. THANK YOU AGAIN!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants