-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[rc2] Fix race condition in SingletonCosmosClientWrapper.Client #36767
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
Conversation
Co-authored-by: AndriySvyryd <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Rewrite the PR summary using this template:
Fixes #{number}
**Description**
{Issue description}
**Customer impact**
{How does the reported issue affects customer's app? Are there workarounds?}
**How found**
{Was it customer reported or found during verification? How many customers are affected?}
**Regression**
{Is it a regression from a released version? Which one?}
**Testing**
{How the changes were tested}
**Risk**
{Low/Medium/High, argument why}
Co-authored-by: AndriySvyryd <[email protected]>
... Done. Rewrote the PR description using the requested template with sections for Description, Customer impact, How found, Regression, Testing, and Risk. |
src/EFCore.Cosmos/Storage/Internal/SingletonCosmosClientWrapper.cs
Outdated
Show resolved
Hide resolved
src/EFCore.Cosmos/Storage/Internal/SingletonCosmosClientWrapper.cs
Outdated
Show resolved
Hide resolved
…nal logic Co-authored-by: AndriySvyryd <[email protected]>
@copilot Use the default emulator URI and key in the |
Co-authored-by: AndriySvyryd <[email protected]>
Done. Updated both files to use the default Cosmos emulator URI (https://localhost:8081) and key (C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==) in the UseCosmos calls. Commit 568b2df |
@copilot Update |
…ide Assert.ThrowsAsync lambda Co-authored-by: AndriySvyryd <[email protected]>
Done. Updated the test to call |
src/EFCore.Cosmos/Storage/Internal/SingletonCosmosClientWrapper.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: roji <[email protected]>
src/EFCore.Cosmos/Storage/Internal/SingletonCosmosClientWrapper.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: roji <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RC2
* Update dependencies from https://github.com/dotnet/dotnet build 283422 Updated Dependencies: Microsoft.DotNet.Arcade.Sdk, Microsoft.DotNet.Build.Tasks.Templating, Microsoft.DotNet.Helix.Sdk (Version 10.0.0-beta.25464.104 -> 10.0.0-beta.25466.101) Microsoft.Extensions.Caching.Memory, Microsoft.Extensions.Configuration, Microsoft.Extensions.Configuration.Abstractions, Microsoft.Extensions.Configuration.EnvironmentVariables, Microsoft.Extensions.Configuration.Json, Microsoft.Extensions.DependencyInjection, Microsoft.Extensions.DependencyModel, Microsoft.Extensions.HostFactoryResolver.Sources, Microsoft.Extensions.Logging, Microsoft.NETCore.App.Ref, Microsoft.NETCore.Platforms, System.Formats.Asn1, System.Runtime.Caching, System.Text.Encodings.Web, System.Text.Json (Version 10.0.0-rc.2.25464.104 -> 10.0.0-rc.2.25466.101) * [rc2] Fix race condition in SingletonCosmosClientWrapper.Client (#36767) Fixes #36714 Co-authored-by: AndriySvyryd <[email protected]> * Fix ExecuteUpdate parameters for multiple properties with same name (#36792) And validate against null required properties in JSON serialization. Fixes #36791 * Stop transforming complex property access to EF.Property (#36786) Fixes #36761 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Jiri Cincura ↹ <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: AndriySvyryd <[email protected]> Co-authored-by: Shay Rojansky <[email protected]>
* Update dependencies from https://github.com/dotnet/dotnet build 283422 Updated Dependencies: Microsoft.DotNet.Arcade.Sdk, Microsoft.DotNet.Build.Tasks.Templating, Microsoft.DotNet.Helix.Sdk (Version 10.0.0-beta.25464.104 -> 10.0.0-beta.25466.101) Microsoft.Extensions.Caching.Memory, Microsoft.Extensions.Configuration, Microsoft.Extensions.Configuration.Abstractions, Microsoft.Extensions.Configuration.EnvironmentVariables, Microsoft.Extensions.Configuration.Json, Microsoft.Extensions.DependencyInjection, Microsoft.Extensions.DependencyModel, Microsoft.Extensions.HostFactoryResolver.Sources, Microsoft.Extensions.Logging, Microsoft.NETCore.App.Ref, Microsoft.NETCore.Platforms, System.Formats.Asn1, System.Runtime.Caching, System.Text.Encodings.Web, System.Text.Json (Version 10.0.0-rc.2.25464.104 -> 10.0.0-rc.2.25466.101) * [rc2] Fix race condition in SingletonCosmosClientWrapper.Client (dotnet#36767) Fixes dotnet#36714 Co-authored-by: AndriySvyryd <[email protected]> * Fix ExecuteUpdate parameters for multiple properties with same name (dotnet#36792) And validate against null required properties in JSON serialization. Fixes dotnet#36791 * Stop transforming complex property access to EF.Property (dotnet#36786) Fixes dotnet#36761 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Jiri Cincura ↹ <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: AndriySvyryd <[email protected]> Co-authored-by: Shay Rojansky <[email protected]>
Fixes #36714
Description
The
SingletonCosmosClientWrapper.Client
property was using non-thread-safe lazy initialization, which caused multipleCosmosClient
instances to be created under concurrent access. This led to resource contention, 100% CPU usage, and 503 errors. The fix moves CosmosClient initialization to the constructor and simplifies the conditional logic for better maintainability.Customer impact
In high-throughput multi-threaded applications this caused resource contention, 100% CPU usage, and 503 errors. There isn't a viable workaround.
How found
Customer reported. This likely affects many EF Cosmos users to some degree but hasn't been reported before as it usually doesn't manifest itself in a reproducible manner.
Regression
No.
Testing
Test added
Risk
Low. Localized change moving from lazy initialization to constructor initialization, which is more straightforward and eliminates thread safety concerns entirely.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.