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

UpdateMergedOptionsFromMicrosoftIdentityOptions Collection was modified; enumeration operation may not execute. #1957

Closed
RbsTechOnline opened this issue Nov 8, 2022 · 45 comments · Fixed by #1979
Labels
bug Something isn't working .NET 7 P1
Milestone

Comments

@RbsTechOnline
Copy link

RbsTechOnline commented Nov 8, 2022

Microsoft.Identity.Web Library

Microsoft.Identity.Web

Microsoft.Identity.Web version

1.25.5

Web app

Sign-in users and call web APIs

Web API

Protected web APIs (validating tokens)

Token cache serialization

Not Applicable

Description

Fist call to web api is causing an error an

Collection was modified; enumeration operation may not execute

in

internal static void UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftIdentityOptions microsoftIdentityOptions, MergedOptions mergedOptions)

Seemingly at

if (!mergedOptions.ClaimActions.Contains(claimAction))

Reproduction steps

This is just a preliminary assessment. Can work towards a full repro if needed

Error message

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at System.Linq.Enumerable.Contains[TSource](IEnumerable`1 source, TSource value, IEqualityComparer`1 comparer)
   at Microsoft.Identity.Web.MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftIdentityOptions microsoftIdentityOptions, MergedOptions mergedOptions)
   at Microsoft.Identity.Web.MicrosoftIdentityWebApiAuthenticationBuilderExtensions.<>c__DisplayClass3_0.<AddMicrosoftIdentityWebApiImplementation>b__0(JwtBearerOptions options, IServiceProvider serviceProvider, IOptionsMonitor`1 mergedOptionsMonitor, IOptionsMonitor`1 msIdOptionsMonitor, IOptions`1 msIdOptions)
   at Microsoft.Extensions.Options.ConfigureNamedOptions`5.Configure(String name, TOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__3`1.<GetOrAdd>b__3_0(String name, ValueTuple`2 arg)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandler`1.InitializeAsync(AuthenticationScheme scheme, HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.GetHandlerAsync(HttpContext context, String authenticationScheme)
   at Microsoft.AspNetCore.Authentication.AuthenticationService.AuthenticateAsync(HttpContext context, String scheme)
   at Microsoft.AspNetCore.Authorization.Policy.PolicyEvaluator.AuthenticateAsync(AuthorizationPolicy policy, HttpContext context)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Serilog.AspNetCore.RequestLoggingMiddleware.Invoke(HttpContext httpContext)
   at NSwag.AspNetCore.Middlewares.SwaggerUiIndexMiddleware.Invoke(HttpContext context)
   at NSwag.AspNetCore.Middlewares.RedirectToIndexMiddleware.Invoke(HttpContext context)
   at NSwag.AspNetCore.Middlewares.OpenApiDocumentMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

Id Web logs

No response

Relevant code snippets

  builder.Services
         .AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
         .AddMicrosoftIdentityWebApi(builder.Configuration.GetSection("IdentityPortal"), "Portal");

  ...

  builder.Services.AddAuthorization();

  ...

  app.UseAuthentication();
  app.UseAuthorization();

  ...

  Controller

  [Authorize(AuthenticationSchemes = "Portal"),RequiredScope("Api.ReadWrite")]

Regression

No response

Expected behavior

I am not really sure where the problem is originating from, or what might be the cause of it.

My questions is, is this known issue, and I am missing something simple?

@RbsTechOnline RbsTechOnline added the question Further information is requested label Nov 8, 2022
@jmprieur
Copy link
Collaborator

jmprieur commented Nov 9, 2022

Thanks @RbsTechOnline
is it an option for you to try out Microsoft.Identity.Web 2.0.5-preview ?
We believe this is fixed there.

@RbsTechOnline
Copy link
Author

@jmprieur it sure is, ill try and report back. Thanks

@jmprieur
Copy link
Collaborator

jmprieur commented Nov 9, 2022

Thanks @RbsTechOnline
cc: @jennyf19

@bartdebever
Copy link

bartdebever commented Nov 10, 2022

Currently running into the same issue, tried the 2.0.5-preview package but the same exception still occurs.
Issue only start occurring after upgrading to .NET 7.

@jennyf19
Copy link
Collaborator

@bartdebever Does the error happen in .NET 6 as well?

@jennyf19
Copy link
Collaborator

@RbsTechOnline and @bartdebever I'm not able to repro on .NET 6 or .NET 7. can you provide exact repro steps, I tried to mimic what @RbsTechOnline did by replicating what he has provided above, but could be missing a key part. I'm running our web app calls web API calls graph sample.

@bartdebever
Copy link

@bartdebever Does the error happen in .NET 6 as well?

This used to work within .NET 6, same package version. I'll attempt to reproduce the issue myself within a new project on GitHub as I, unfortunately, can't share the source for the actual project.

@RbsTechOnline
Copy link
Author

Hi @jennyf19 I can verify the same error in Microsoft.Identity.Web 2.0.5-preview

I do note the stack trace is slightly different,

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at System.Linq.Enumerable.Contains[TSource](IEnumerable`1 source, TSource value, IEqualityComparer`1 comparer)
   at Microsoft.Identity.Web.MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftIdentityOptions microsoftIdentityOptions, MergedOptions mergedOptions)
   at Microsoft.Identity.Web.MicrosoftIdentityOptionsMerger.PostConfigure(String name, MicrosoftIdentityOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__3`1.<GetOrAdd>b__3_0(String name, ValueTuple`2 arg)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.Identity.Web.MicrosoftIdentityWebApiAuthenticationBuilderExtensions.<>c__DisplayClass3_0.<AddMicrosoftIdentityWebApiImplementation>b__2(JwtBearerOptions options, IServiceProvider serviceProvider, IOptionsMonitor`1 mergedOptionsMonitor, IOptionsMonitor`1 msIdOptionsMonitor)
   at Microsoft.Extensions.Options.ConfigureNamedOptions`4.Configure(String name, TOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__3`1.<GetOrAdd>b__3_0(String name, ValueTuple`2 arg)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandler`1.InitializeAsync(AuthenticationScheme scheme, HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.GetHandlerAsync(HttpContext context, String authenticationScheme)
   at Microsoft.AspNetCore.Authentication.AuthenticationService.AuthenticateAsync(HttpContext context, String scheme)
   at Microsoft.AspNetCore.Authorization.Policy.PolicyEvaluator.AuthenticateAsync(AuthorizationPolicy policy, HttpContext context)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Serilog.AspNetCore.RequestLoggingMiddleware.Invoke(HttpContext httpContext)
   at NSwag.AspNetCore.Middlewares.SwaggerUiIndexMiddleware.Invoke(HttpContext context)
   at NSwag.AspNetCore.Middlewares.RedirectToIndexMiddleware.Invoke(HttpContext context)
   at NSwag.AspNetCore.Middlewares.OpenApiDocumentMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

I cant seem to find the 2.0.5-preview tag to download the source.

However, as soon as i get a chance, I'll try to get repro if one isn't already tendered

@Eneuman
Copy link

Eneuman commented Nov 11, 2022

We have the same issue

@evandromendonca
Copy link

Hi guys, I am having the same issue here. Did anyone find something around it?

@jmprieur
Copy link
Collaborator

@bartdebever, @bartdebever @Eneuman @RbsTechOnline @evandromendonca
Does one of you has repro steps?

Do you configure claims actions?

@evandromendonca
Copy link

Hi guys, hi @jmprieur.

Unfortunately, I don't have repo with repro steps. But I could narrow down the issue to Swagger "Swashbuckle.AspNetCore".

The issue was that the UseSwagger code must be after the UseAuthentication and UseAuthorization for some reason. I had it the other way around.

So this did the trick for me:

app.UseAuthentication();
app.UseAuthorization();
// and then:
app.UseSwagger();
app.UseSwaggerUI();

I didn't have the time to drill down exactly what was causing the issue due to tight deadlines here, but if it is still a thing I can try in the future.

I hope this can help you guys.

Cheers!

@billyjacobs2014
Copy link

billyjacobs2014 commented Nov 13, 2022

I was experiencing the same problem consistently after upgrading from .net 6 to .net 7.

I have a React client with a .net 7 api. When the application loaded the first time by entering in the URL into the browser and hitting the enter key it all worked as expected. If I then click the browser refresh button, I was getting the same error above. As long as I hit the refresh button, I would get the same error. If I typed the URL into the address bar and hit enter after it errored, it would work as expected. Not sure why that would trigger the error.

Also, I thought it was swagger also initially, but I commented that all out and was still receiving the error.

I then upgraded to Microsoft.Identity.Web 2.0.5-preview and I thought the problem was resolved but it was not.

After upgrading I can still reproduce the error if I start the api, and right after visual studio appears to be running the api, but the api is not actually ready according to the console (i.e. run api as Output Type = Console Application) and then refresh the browser before the api is ready in the console, then I get the error. If I then type in the url manually it works and then refreshing works after.

It appears to be an issue only if api is not really ready to take requests even though visual studio appears to have started up. Viewing the console in this case is helpful. I have not downgraded to Microsoft.Identity.Web 1.25.5 to see if it behaves the same with the previous version.

UPDATE: I downgraded back to 1.25.5, and reverted everything back to what it was just after upgrading to .net 7.0 and the issue only happens when I attempt to reload the application when the api is not ready as described above. If I wait for everything to load in the console including listing all of the entities for EF warnings, then it works as expected with Microsoft.Identity.Web 1.25.5 .

jennyf19 added a commit that referenced this issue Nov 13, 2022
@jennyf19
Copy link
Collaborator

jennyf19 commented Nov 13, 2022

Thanks for all the suggestions and repro steps. Does anyone want to see if this branch resolves the issue? I'm still working on a repro of the issue.
@RbsTechOnline @billyjacobs2014 @bartdebever @evandromendonca @Eneuman

@magols
Copy link

magols commented Nov 14, 2022

Thanks for all the suggestions and repro steps. Does anyone want to see if this branch resolves the issue?

@jennyf19 I have notable problems with this in development of a Serverside Blazor and and mostly when app is restarted during hot reload.

Using the branch https://github.com/AzureAD/microsoft-identity-web/tree/jennyf/fix1957 I still have the issue unfortunately.

I am not well versed in this type of code but I noted that the (!mergedOptions.ClaimActions.Contains(claimAction)) does not behave as seemingly expected as it will always evaluate to false. There is always a call to Add() duplicates into mergedOptions.ClaimActions.

As illustrated by this screenshot with duplicate DeleteClaimActions with the same ClaimType and ValueType.

image

@Eneuman
Copy link

Eneuman commented Nov 14, 2022

I think contains will check if the exact instance exists in the collection so if any claims mapping has been done, this will probably fail. A more robust implementation would be to check if the claim type and value exists.

@jennyf19
Copy link
Collaborator

@magols and @Eneuman the branch has been updated, are the issues resolved? I still cannot manage to repro it, so in order to determine a different fix, I will need clear repro steps. Thanks

@magols
Copy link

magols commented Nov 15, 2022

@jennyf19 the good news is that the fix no longer produces an error merging the ClaimsActions here:

The bad news is that we just moved the same type of error to line 214 and the merging of Scopes

foreach (var scope in microsoftIdentityOptions.Scope)
{
if (!string.IsNullOrWhiteSpace(scope) && !mergedOptions.Scope.Any(s => string.Equals(s, scope, StringComparison.OrdinalIgnoreCase)))
{
mergedOptions.Scope.Add(scope);
}
}

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
   at System.Collections.Generic.HashSet`1.Enumerator.MoveNext()
   at System.Linq.Enumerable.Any[TSource](IEnumerable`1 source, Func`2 predicate)
   at Microsoft.Identity.Web.MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftIdentityOptions microsoftIdentityOptions, MergedOptions mergedOptions) in C:\Source\microsoft-identity-web\src\Microsoft.Identity.Web.TokenAcquisition\MergedOptions.cs:line 212
   at Microsoft.Identity.Web.MicrosoftIdentityOptionsMerger.PostConfigure(String name, MicrosoftIdentityOptions options) in C:\Source\microsoft-identity-web\src\Microsoft.Identity.Web.TokenAcquisition\OptionsMergers\MicrosoftIdentityOptionsMerger.cs:line 19
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__3`1.<GetOrAdd>b__3_0(String name, ValueTuple`2 arg)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.Identity.Web.MicrosoftIdentityWebAppAuthenticationBuilderExtensions.<>c__DisplayClass5_0.<AddMicrosoftIdentityWebAppInternal>b__3(OpenIdConnectOptions options, IServiceProvider serviceProvider, IOptionsMonitor`1 mergedOptionsMonitor, IOptionsMonitor`1 msIdOptionsMonitor, IOptions`1 msIdOptions) in C:\Source\microsoft-identity-web\src\Microsoft.Identity.Web\WebAppExtensions\MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs:line 287
   at Microsoft.Extensions.Options.ConfigureNamedOptions`5.Configure(String name, TOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__3`1.<GetOrAdd>b__3_0(String name, ValueTuple`2 arg)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandler`1.InitializeAsync(AuthenticationScheme scheme, HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.GetHandlerAsync(HttpContext context, String authenticationScheme)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

@magols
Copy link

magols commented Nov 15, 2022

I think I am able to force the error to occur in a brand new Blazor application. I don't know if this is considered "clear repro steps" but...

  1. Create a new .NET 7 Serverside Blazor app with Microsoft Identity Platform.
  2. Start app in a terminal with dotnet watch run, with a few duplicate browser tabs opened. (I have one Incognito window logged in also)
  3. In terminal press Ctrl-R to restart application and to reload tabs. It won't crash every time but usually after a few (4-5) restarts the error will occur in at least one of the browser tabs.

@jennyf19
Copy link
Collaborator

@magols Those steps helped a lot. I have updated the branch.

jennyf19 added a commit that referenced this issue Nov 15, 2022
@billyjacobs2014
Copy link

billyjacobs2014 commented Nov 15, 2022

This fix is not on a branch according to git. Basically anytime any of my APIs shut down for inactivity and then restart, the error ALWAYS occurs. It happens always on initial load of the api. It goes away after that but as soon as it shuts down (for inactivity) and restarts, it happens again.

I know you are trying to fix this but does Microsoft not place any priority on this library? Is this not a fundamental piece of their software? This is the second major release (6,7) where I have had to deal with the Microsoft Identity libraries. Right now this issue should have had as many people rushing to fix it as fast as possible. If I did this to my clients they would all fire me. Right now I am going to spend the next couple of hours downgrading to .net 6 to resolve this for them and it would be nice if Microsoft would put the same effort for libraries that are fundamental to their clients.

When 6 came out, Maui with IOS would not work with Microsoft.Identity.Client for months. My only option in this case is to downgrade back to 6 until this issue is resolved. I find it hard to believe that no one has seen this issue before releasing .net 7, as it occurs in 4 of my api projects and is easily reproducible. I can send you my production logs to show you if you like but it is essentially the same error as reported above.

@jmprieur
Copy link
Collaborator

jmprieur commented Nov 15, 2022

@billyjacobs2014 what do you mean not on a branch? We worked together on it yesterday night with Jenny. We have been prioritizing fixing it, but had not been able to repro it until @magols's provided clear repro steps

The fix is in jennyf/fix1957.
Does the issue still repro when you use packages that you build from that branch? We'd like to have a confirmation that the issue is fixed.
We aim to do a new release (1.25.6) today.

@jennyf19
Copy link
Collaborator

The issue is fixed in 1.25.6 based on the repro provided by @magols.

@jennyf19 jennyf19 added this to the 1.25.6 milestone Nov 16, 2022
@jennyf19 jennyf19 added the bug Something isn't working label Nov 16, 2022
@f1nzer
Copy link

f1nzer commented Nov 16, 2022

@jennyf19
I've created a simple project where you can reproduce the problem. There is a single unit test that should pass.
https://github.com/f1nzer/ms-identity-web-1957-bug-repro

@MGD-dev
Copy link

MGD-dev commented Nov 16, 2022

I was having the same issue on my API for a Blazor WASM project after updating to .net 7.
Oddly enough I had updated 2 projects and only 1 was having the issue.
The project that was not having the issue makes a single API call on startup before any other calls can be made to gather user permissions.
For me and as @magols stated it seems the issue occurs when multiple API calls are made on startup.
I updated the 2nd project to make the single service call before any others could be made and it fixed the issue.

@jmprieur jmprieur added P1 and removed fixed labels Nov 16, 2022
@jmprieur
Copy link
Collaborator

jmprieur commented Nov 18, 2022

We believe we have a fix, which we'll release tomorrow PST.
Thanks f1nzer for your repro, which was instrumental.

@jennyf19
Copy link
Collaborator

+1 to @f1nzer

@jennyf19
Copy link
Collaborator

1.25.7 is released. We will release the fix in v2-preview as well.

@f1nzer
Copy link

f1nzer commented Nov 21, 2022

@jennyf19 @jmprieur
Unfortunately, the issue was not fixed. The problem might be reproduced on the same test using v1.25.8, just run it a couple of times.
Stacktrace:

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
   at System.Collections.Generic.HashSet`1.AddIfNotPresent(T value, Int32& location)
   at System.Collections.Generic.HashSet`1.System.Collections.Generic.ICollection<T>.Add(T item)
   at Microsoft.Identity.Web.MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftIdentityOptions microsoftIdentityOptions, MergedOptions mergedOptions)
   at Microsoft.Identity.Web.MicrosoftIdentityWebApiAuthenticationBuilderExtensions.<>c__DisplayClass3_0.<AddMicrosoftIdentityWebApiImplementation>b__0(JwtBearerOptions options, IServiceProvider serviceProvider, IOptionsMonitor`1 mergedOptionsMonitor, IOptionsMonitor`1 msIdOptionsMonitor, IOptions`1 msIdOptions)
   at Microsoft.Extensions.Options.ConfigureNamedOptions`5.Configure(String name, TOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__3`1.<GetOrAdd>b__3_0(String name, ValueTuple`2 arg)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandler`1.InitializeAsync(AuthenticationScheme scheme, HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.GetHandlerAsync(HttpContext context, String authenticationScheme)
   at Microsoft.AspNetCore.Authentication.AuthenticationService.AuthenticateAsync(HttpContext context, String scheme)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context)

@jmprieur
Copy link
Collaborator

jmprieur commented Nov 21, 2022

which same test @f1nzer? you mean the unit test? or the test with dotnet watch?

@slubowsky
Copy link

@jennyf19 @jmprieur Just FYI, in my API, it used to happen (beginning after upgrade to .NET 7) every time it started up (when hit first from web app - multiple connections at once, no issues when hit first from single swagger connection, as documented above)
Using 2.0.6-preview, it now works about 50% of the time. So not sure if that's better or worse, but clearly there is still a problem.

System.InvalidOperationException: 'Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.'

This exception was originally thrown at this call stack:
    System.ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported() in ThrowHelper.cs
    System.Collections.Generic.HashSet<T>.AddIfNotPresent(T, out int) in HashSet.cs
    System.Collections.Generic.HashSet<T>.System.Collections.Generic.ICollection<T>.Add(T) in HashSet.cs
    Microsoft.Identity.Web.MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions(Microsoft.Identity.Web.MicrosoftIdentityOptions, Microsoft.Identity.Web.MergedOptions) in MergedOptions.cs
    Microsoft.Identity.Web.MicrosoftIdentityOptionsMerger.PostConfigure(string, Microsoft.Identity.Web.MicrosoftIdentityOptions) in MicrosoftIdentityOptionsMerger.cs
    Microsoft.Extensions.Options.OptionsFactory<TOptions>.Create(string) in OptionsFactory.cs
    Microsoft.Extensions.Options.OptionsCache<TOptions>.GetOrAdd.AnonymousMethod__3_0(string, (System.Func<string, TArg, TOptions> createOptions, TArg factoryArgument)) in OptionsCache.cs
    System.Collections.Concurrent.ConcurrentDictionary<TKey, TValue>.GetOrAdd<TArg>(TKey, System.Func<TKey, TArg, TValue>, TArg) in ConcurrentDictionary.cs
    Microsoft.Extensions.Options.OptionsCache<TOptions>.GetOrAdd<TArg>(string, System.Func<string, TArg, TOptions>, TArg) in OptionsCache.cs
    Microsoft.Extensions.Options.OptionsMonitor<TOptions>.Get(string) in OptionsMonitor.cs
    ...
    [Call Stack Truncated]

@f1nzer
Copy link

f1nzer commented Nov 21, 2022

which same test @f1nzer? you mean the unit test? or the test with dotnet watch?

It can be reproduced with the same unit test I sent you before.

@jennyf19
Copy link
Collaborator

@f1nzer we have a fix in master now, I'm not able to repro it with the test you provided, even with 100k concurrent calls.

@bartdebever
Copy link

I want to confirm that as of the latest release (1.25.9), this issue has indeed been fixed and working in a production scenario.

@jmprieur
Copy link
Collaborator

Thanks for confirming @bartdebever
and thanks everybody for your help!

@petedishman
Copy link

The linked merged request #1979 is still open and not merged to master yet? And the source code download for the 1.25.9 release doesn't show it either.

Has this actually not been resolved in the latest nuget release or have I misunderstood something?

@jmprieur jmprieur modified the milestones: 1.25.6, 1.26.9 Dec 2, 2022
jmprieur added a commit that referenced this issue Dec 2, 2022
Replace IOptionsMonitor<MergedOptions> by
IMergedOptionsStore.

See also #1979 for master (v1.0)
(this PR is for rel/v2.0)
jmprieur added a commit that referenced this issue Dec 2, 2022
Replace IOptionsMonitor<MergedOptions> by
IMergedOptionsStore.

See also #1979 for master (v1.0)
(this PR is for rel/v2.0)
@rondelward-pf
Copy link

I updated to version 1.25.10 and I can still see this error being thrown in the logs that we have on a .NET6 API application. Is this a regression or am I missing something? (runtime 6.0.13). Stack trace looks identical to the original issue

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
  ?, in bool HashSet<T>.AddIfNotPresent(T value, out int location)
  ?, in void HashSet<T>.System.Collections.Generic.ICollection<T>.Add(?)
  ?, in void MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftIdentityOptions microsoftIdentityOptions, MergedOptions mergedOptions)
  ?, in void MicrosoftIdentityWebAppAuthenticationBuilderExtensions.AddMicrosoftIdentityWebAppInternal(AuthenticationBuilder builder, Action<MicrosoftIdentityOptions> configureMicrosoftIdentityOptions, Action<CookieAuthenticationOptions> configureCookieAuthenti...
  ?, in void ConfigureNamedOptions<TOptions, TDep1, TDep2, TDep3, TDep4>.Configure(string name, TOptions options)
  ?, in TOptions OptionsFactory<TOptions>.Create(string name)
  ?, in TOptions OptionsMonitor<TOptions>.Get(string name)+(string name, IOptionsFactory<TOptions> factory) => { }
  ?, in TOptions OptionsCache<TOptions>.GetOrAdd<TArg>(string name, Func<string, TArg, TOptions> createOptions, TArg factoryArgument)+(string name,
  ?, in TValue ConcurrentDictionary<TKey, TValue>.GetOrAdd<TArg>(TKey key, Func<TKey, TArg, TValue> valueFactory, TArg factoryArgument)
  ?, in TOptions OptionsCache<TOptions>.GetOrAdd<TArg>(string name, Func<string, TArg, TOptions> createOptions, TArg factoryArgument)
  ?, in TOptions OptionsMonitor<TOptions>.Get(string name)
  ?, in async Task AuthenticationHandler<TOptions>.InitializeAsync(AuthenticationScheme scheme, HttpContext context)
  ?, in async Task<IAuthenticationHandler> AuthenticationHandlerProvider.GetHandlerAsync(HttpContext context, string authenticationScheme)
  ?, in async Task AuthenticationMiddleware.Invoke(HttpContext context)

<PackageReference Include="Microsoft.Identity.Web" Version="1.25.10" />

Should I open a new issue for this? I can maybe try downgrading to 1.25.9 but I'm quite confused. Is the fix only available in .NET7?

@jmprieur
Copy link
Collaborator

jmprieur commented Feb 1, 2023

the .NET 7 fix should be released mid February.
Meanwhile, a workaround is described here: #1995 (comment)

@rondelward-pf
Copy link

Thanks @jmprieur, I'll implement the suggested workaround for now. Will the fix also make it to the LTS .NET6 version as well? I thought it was a package problem not in the runtime but just curious if I need to wait for .NET7 or 8 to remove the workaround.

@jmprieur
Copy link
Collaborator

jmprieur commented Feb 2, 2023

@rondelward-pf : it's a regression in .NET 7.
So I believe the next runtime patch of .NET 7 should fix it.

@AndreErb
Copy link

AndreErb commented Sep 26, 2023

@jmprieur
We're still seeing this in multiple APIs, though using "Microsoft.Identity.Web" V2.13.4
Also, we are using .NET 6, only.

You wrote, that's a regression in .NET 7, but you did not specifically anwser the question of @rondelward-pf, if this fixes it for .NET 6, too .

What confuses me too, is that the Fix was made in "Microsoft.Identity.Web", so why did you point out the next .NET 7 runtime patch? Shouldn't the commit fix it by itself?

So I believe the next runtime patch of .NET 7 should fix it.

This is one of these log entries, using V2.13.4

2023-09-21T13:07:16.4158570+02:00 [Thread:] [ERR] (Microsoft.AspNetCore.Server.IIS.Core.IISHttpServer) Connection ID ""5188146776636391632"", Request ID ""800000d1-0001-4800-b63f-84710c7967bb"": An unhandled exception was thrown by the application.
System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
   at System.Collections.Generic.HashSet`1.AddIfNotPresent(T value, Int32& location)
   at System.Collections.Generic.HashSet`1.System.Collections.Generic.ICollection<T>.Add(T item)
   at Microsoft.Identity.Web.MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftIdentityOptions microsoftIdentityOptions, MergedOptions mergedOptions)
   at Microsoft.Identity.Web.MicrosoftIdentityOptionsMerger.PostConfigure(String name, MicrosoftIdentityOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsMonitor`1.<>c.<Get>b__10_0(String name, IOptionsFactory`1 factory)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__3`1.<GetOrAdd>b__3_0(String name, ValueTuple`2 arg)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.Identity.Web.MicrosoftIdentityWebApiAuthenticationBuilderExtensions.<>c__DisplayClass3_0.<AddMicrosoftIdentityWebApiImplementation>b__2(JwtBearerOptions options, IServiceProvider serviceProvider, IMergedOptionsStore mergedOptionsMonitor, IOptionsMonitor`1 msIdOptionsMonitor)
   at Microsoft.Extensions.Options.ConfigureNamedOptions`4.Configure(String name, TOptions options)
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsMonitor`1.<>c.<Get>b__10_0(String name, IOptionsFactory`1 factory)
   at Microsoft.Extensions.Options.OptionsCache`1.<>c__3`1.<GetOrAdd>b__3_0(String name, ValueTuple`2 arg)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd[TArg](String name, Func`3 createOptions, TArg factoryArgument)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandler`1.InitializeAsync(AuthenticationScheme scheme, HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.GetHandlerAsync(HttpContext context, String authenticationScheme)
   at Microsoft.AspNetCore.Authentication.AuthenticationService.AuthenticateAsync(HttpContext context, String scheme)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.<Invoke>g__AwaitMatcher|8_0(EndpointRoutingMiddleware middleware, HttpContext httpContext, Task`1 matcherTask)
   at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Server.IIS.Core.IISHttpContextOfT`1.ProcessRequestAsync()

@Bigcountry409

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working .NET 7 P1
Projects
None yet