Skip to content

Conversation

@henriquewr
Copy link
Contributor

Added sync for lazy loading

Fixes #35528

 - Added sync for lazy loading

Fixes #35528
@henriquewr henriquewr requested a review from a team as a code owner January 25, 2025 03:45
@henriquewr
Copy link
Contributor Author

@dotnet-policy-service agree

@AndriySvyryd AndriySvyryd self-assigned this Jan 25, 2025
@roji roji changed the title Fixed #35528 Fixed lazy loading thread safety Jan 25, 2025
Added tests for lazy loading thread safety
@AndriySvyryd AndriySvyryd requested a review from Copilot January 29, 2025 19:19
Copy link

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.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

@AndriySvyryd
Copy link
Member

@henriquewr Could you explain what the issue in the original code was? It seems that it made some incorrect assumption, but I don't see what it was exactly

@AndriySvyryd
Copy link
Member

Oh, I see. While it prevented a second thread from loading it didn't make it wait for the first thread to finish.

@henriquewr
Copy link
Contributor Author

@AndriySvyryd Exacly that what i was going to say


if (exists)
{
if (isLoadingValue.Depth.Value == 1)
Copy link
Member

Choose a reason for hiding this comment

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

This is an interesting optimization. It definitely warrants a comment on how it works as the next person to read this code will be very confused.

@roji @cincuranet What's your opinion of this usage of AsyncLocal?

@AndriySvyryd AndriySvyryd merged commit a149ce7 into dotnet:main Feb 1, 2025
4 of 6 checks passed
@AndriySvyryd
Copy link
Member

Thanks for your contribution!

@henriquewr
Copy link
Contributor Author

You're welcome, I'm happy to contribute

@henriquewr henriquewr deleted the Issue35528 branch February 1, 2025 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lazy loading is not thread safe

2 participants