Skip to content

Conversation

@msJinLei
Copy link
Contributor

@msJinLei msJinLei commented Mar 30, 2025

What to fix

  • In the method RegisterComponent, the operation that checks key existance _componentRegistry.TryGetValue(key, out var existed)and the operation that updates the value of the key _componentRegistry[key] = component are not atomic. The result is the when you want to register the components with the same key in different threads in no-overwrite mode, the component is actually overwritten. What you can see is that with the unfixed codes, the result of the test case TestComponentRegistrationSameComponentNoOverwritten is unpredictable.

Add the test cases for component registeration
@msJinLei msJinLei force-pushed the jinlei/componentRegistry branch from 2b81798 to 10ae7e0 Compare March 30, 2025 17:43
@msJinLei msJinLei requested a review from Copilot March 30, 2025 18:10

This comment was marked as outdated.

@msJinLei msJinLei requested a review from Copilot March 30, 2025 18:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a concurrency issue in the component registration for AzureSession, ensuring that component initialization and event handler updates occur atomically during concurrent operations.

  • Refactored component registration logic to use AddOrUpdate to improve thread-safety.
  • Updated event handler registration and unregistration for handling listener components.
  • Added extensive tests to validate different component registration and unregistration scenarios.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Authentication.Abstractions/AzureSession.cs Updated RegisterComponent method to atomically update components and correctly manage event subscriptions under concurrency
src/Authentication.Abstractions.Test/AzureSessionTest.cs Added tests covering parallel registration, unregistration, and event handler invocation scenarios

@msJinLei msJinLei requested review from NoriZC and isra-fel March 30, 2025 18:54
@msJinLei msJinLei changed the title Fix concurrency issue of Component Registery o Azure Session Fix concurrency issue of Component Registery of Azure Session Apr 1, 2025
isra-fel
isra-fel previously approved these changes Apr 1, 2025
@msJinLei msJinLei requested a review from NoriZC April 1, 2025 06:11
@msJinLei msJinLei merged commit c5a88dc into main Apr 1, 2025
2 checks passed
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.

4 participants