Skip to content

Fix rare concurrency issue in back-office auth middleware #19418

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

Merged

Conversation

kjac
Copy link
Contributor

@kjac kjac commented May 26, 2025

Prerequisites

  • I have added steps to test this contribution in the description below

Description

It is apparently possible to trigger a concurrency exception in the BackOfficeAuthorizationInitializationMiddleware.

I have not been able to reproduce it, but here's a screenshot of the error (sent by a colleague):

image

This PR fixes it. I'm erring on the side of caution here, since the PR targets V16 RC. It could likely be solved in a different way using ConcurrentBag<string> instead of HashSet<string>() to store the known hosts, but since we still need to perform explicit locking to avoid duplicate updates of back-office hosts, I think this is an OK solution.

Testing this PR

As mentioned, I can't reproduce the issue myself. So most of all, a visual inspection of the code is in order - and then a verification that the back-office still works 😆

Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and correct to solve the reported issue. As noted, manual testing can't be much more than loading up the backoffice and logging in - but I've done what without seeing any issues.

@AndyButland AndyButland merged commit 407e7a2 into release/16.0 May 26, 2025
21 of 22 checks passed
@AndyButland AndyButland deleted the v16/improvement/middleware-concurrency-handling branch May 26, 2025 10:25
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

Successfully merging this pull request may close these issues.

2 participants