Logout Notifications can result in concurrent access to underlying DbContext

[mackie1001]

Which version of Duende IdentityServer are you using?
7.0.7

Which version of .NET are you using?
8.0.8

Describe the bug
We've observed in our logs occasional exceptions being thrown from within DefaultBackChannelLogoutService.SendLogoutNotificationsAsync that relate to multi-threaded access to the operational store DbContext (see stack trace below).
This appears to be due to that method doing a Task.WhenAll for the collection of logout requests and the code that generates the logout token uses the key management implementation with sometimes needs to fetch the key material from the DB. The concurrent executions all share the instance DbContext and thus the error.

To Reproduce
Observed under production load but theoretically:

• Have a session with multiple clients with back channel endpoints registered
• Wait until the key management implementation cache expires
• Execute DefaultBackChannelLogoutService.SendLogoutNotificationsAsync for said session

Expected behavior

The implementation is either changed to await each request in turn or the scoping of the underlying services is revised to avoid this situation when executing requests in parallel.

Log output/exception with stacktrace

System.InvalidOperationException: A second operation was started on this context instance before a previous operation completed. This is usually caused by different threads concurrently using the same instance of DbContext. For more information on how to avoid threading issues with DbContext, see https://go.microsoft.com/fwlink/?linkid=2097913.\r\n at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable1.AsyncEnumerator.MoveNextAsync()\r\n at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable1 source, CancellationToken cancellationToken)\r\n at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable1 source, CancellationToken cancellationToken)\r\n at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToArrayAsync[TSource](IQueryable1 source, CancellationToken cancellationToken)\r\n at Duende.IdentityServer.EntityFramework.Stores.SigningKeyStore.LoadKeysAsync() in //src/EntityFramework.Storage/Stores/SigningKeyStore.cs:line 64\r\n at Duende.IdentityServer.Services.KeyManagement.KeyManager.GetAllKeysFromStoreAsync(Boolean cache) in //src/IdentityServer/Services/Default/KeyManagement/KeyManager.cs:line 428\r\n at Duende.IdentityServer.Services.KeyManagement.KeyManager.GetAllKeysInternalAsync() in //src/IdentityServer/Services/Default/KeyManagement/KeyManager.cs:line 108\r\n at Duende.IdentityServer.Services.KeyManagement.KeyManager.GetCurrentKeysAsync() in //src/IdentityServer/Services/Default/KeyManagement/KeyManager.cs:line 72\r\n at Duende.IdentityServer.Services.KeyManagement.AutomaticKeyManagerKeyStore.GetAllSigningCredentialsAsync() in //src/IdentityServer/Services/Default/KeyManagement/AutomaticKeyManagerKeyStore.cs:line 92\r\n at Duende.IdentityServer.Services.KeyManagement.AutomaticKeyManagerKeyStore.GetSigningCredentialsAsync() in //src/IdentityServer/Services/Default/KeyManagement/AutomaticKeyManagerKeyStore.cs:line 78\r\n at Duende.IdentityServer.Services.DefaultKeyMaterialService.GetSigningCredentialsAsync(IEnumerable1 allowedAlgorithms) in /_/src/IdentityServer/Services/Default/DefaultKeyMaterialService.cs:line 60\r\n at Duende.IdentityServer.Services.DefaultTokenCreationService.CreateJwtAsync(Token token, String payload, Dictionary2 headerElements) in //src/IdentityServer/Services/Default/DefaultTokenCreationService.cs:line 130\r\n at Access.Identity.Services.IdentityServer.TokenCreationService.CreateTokenAsync(Token token) in X:\agent\17\s\src\Access.Identity.Services\IdentityServer\TokenCreationService.cs:line 46\r\n at Access.Identity.Services.IdentityServer.BackChannelLogoutService.CreateTokenAsync(BackChannelLogoutRequest request) in X:\agent\17\s\src\Access.Identity.Services\IdentityServer\BackChannelLogoutService.cs:line 62\r\n at Duende.IdentityServer.Services.DefaultBackChannelLogoutService.CreateFormPostPayloadAsync(BackChannelLogoutRequest request) in //src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs:line 133\r\n at Duende.IdentityServer.Services.DefaultBackChannelLogoutService.SendLogoutNotificationAsync(BackChannelLogoutRequest request) in //src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs:line 111\r\n at Access.Identity.Services.IdentityServer.BackChannelLogoutService.SendLogoutNotificationAsync(BackChannelLogoutRequest request) in X:\agent\17\s\src\Access.Identity.Services\IdentityServer\BackChannelLogoutService.cs:line 71\r\n at Duende.IdentityServer.Services.DefaultBackChannelLogoutService.SendLogoutNotificationsAsync(LogoutNotificationContext context) in //src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs:line 89\r\n at Access.Identity.Services.UserSessionBackChannelLogoutService.DoBackChannelSignoutsForSession(PerformContext context, SessionDescriptor session) in X:\agent\17\s\src\Access.Identity.Services\UserSessionBackChannelLogoutService.cs:line 87\r\n at InvokeStub_TaskAwaiter.GetResult(Object, Object, IntPtr*)\r\n at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Additional context

We noticed an increased prevalence of this issue since upgrading to .Net 8. We also had a similar bug in our own code that presented itself much more under the same production load since upgrading. We're putting this down to runtime changes increasing the chance of genuine parallelism when using the Task.WhenAll pattern.

[RolandGuijt]

@mackie1001 I've moved this to the IdentityServer repo so that it can be picked up by our team to be fixed.
We'll have to triage it internally first.

[RolandGuijt]

@mackie1001 As a short term workaround you could override SendLogoutNotificationsAsync in the DefaultBackChannelLogoutService class and execute the tasks one by one.

[mackie1001]

@RolandGuijt Thanks, as it stands now our retry mechanism seems to handle it eventually but we'll consider overriding the implementation in the meantime. Thanks for looking into it.