Skip to content

Commit 0126da8

Browse files
[rc2] Fix race condition in SingletonCosmosClientWrapper.Client (#36767)
Fixes #36714 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
1 parent afb65e7 commit 0126da8

File tree

5 files changed

+49
-25
lines changed

5 files changed

+49
-25
lines changed

src/EFCore.Cosmos/Metadata/Conventions/Internal/CosmosConventionSetBuilder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ private static IServiceScope CreateServiceScope()
8282
var serviceProvider = new ServiceCollection()
8383
.AddEntityFrameworkCosmos()
8484
.AddDbContext<DbContext>((p, o) =>
85-
o.UseCosmos("localhost", "_", "_")
85+
o.UseCosmos("https://localhost:8081", "C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==", "_")
8686
.UseInternalServiceProvider(p))
8787
.BuildServiceProvider();
8888

src/EFCore.Cosmos/Storage/Internal/SingletonCosmosClientWrapper.cs

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,7 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal;
1616
public class SingletonCosmosClientWrapper : ISingletonCosmosClientWrapper
1717
{
1818
private static readonly string UserAgent = " Microsoft.EntityFrameworkCore.Cosmos/" + ProductInfo.GetVersion();
19-
private readonly CosmosClientOptions _options;
20-
private readonly string? _endpoint;
21-
private readonly string? _key;
22-
private readonly string? _connectionString;
23-
private readonly TokenCredential? _tokenCredential;
24-
private CosmosClient? _client;
19+
private readonly CosmosClient _client;
2520

2621
/// <summary>
2722
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -31,10 +26,6 @@ public class SingletonCosmosClientWrapper : ISingletonCosmosClientWrapper
3126
/// </summary>
3227
public SingletonCosmosClientWrapper(ICosmosSingletonOptions options)
3328
{
34-
_endpoint = options.AccountEndpoint;
35-
_key = options.AccountKey;
36-
_connectionString = options.ConnectionString;
37-
_tokenCredential = options.TokenCredential;
3829
var configuration = new CosmosClientOptions { ApplicationName = UserAgent, Serializer = new JsonCosmosSerializer() };
3930

4031
if (options.Region != null)
@@ -97,7 +88,13 @@ public SingletonCosmosClientWrapper(ICosmosSingletonOptions options)
9788
configuration.HttpClientFactory = options.HttpClientFactory;
9889
}
9990

100-
_options = configuration;
91+
_client = options switch
92+
{
93+
{ ConnectionString: not null and not "" } => new CosmosClient(options.ConnectionString, configuration),
94+
{ TokenCredential: not null } => new CosmosClient(options.AccountEndpoint, options.TokenCredential, configuration),
95+
{ AccountEndpoint: not null } => new CosmosClient(options.AccountEndpoint, options.AccountKey, configuration),
96+
_ => throw new InvalidOperationException(CosmosStrings.ConnectionInfoMissing)
97+
};
10198
}
10299

103100
/// <summary>
@@ -106,14 +103,7 @@ public SingletonCosmosClientWrapper(ICosmosSingletonOptions options)
106103
/// any release. You should only use it directly in your code with extreme caution and knowing that
107104
/// doing so can result in application failures when updating to a new Entity Framework Core release.
108105
/// </summary>
109-
public virtual CosmosClient Client
110-
=> _client ??= string.IsNullOrEmpty(_connectionString)
111-
? _tokenCredential == null
112-
? _endpoint == null
113-
? throw new InvalidOperationException(CosmosStrings.ConnectionInfoMissing)
114-
: new CosmosClient(_endpoint, _key, _options)
115-
: new CosmosClient(_endpoint, _tokenCredential, _options)
116-
: new CosmosClient(_connectionString, _options);
106+
public virtual CosmosClient Client => _client;
117107

118108
/// <summary>
119109
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -123,7 +113,6 @@ public virtual CosmosClient Client
123113
/// </summary>
124114
public virtual void Dispose()
125115
{
126-
_client?.Dispose();
127-
_client = null;
116+
_client.Dispose();
128117
}
129118
}

test/EFCore.Cosmos.FunctionalTests/ConfigPatternsCosmosTest.cs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Collections.Concurrent;
45
using Microsoft.Azure.Cosmos;
56

67
// ReSharper disable UnusedAutoPropertyAccessor.Local
@@ -124,6 +125,41 @@ public async Task Should_throw_if_specified_connection_mode_is_wrong()
124125
});
125126
}
126127

128+
[ConditionalFact]
129+
public async Task Cosmos_client_instance_is_thread_safe()
130+
{
131+
await using var testDatabase = await CosmosTestStore.CreateInitializedAsync(DatabaseName);
132+
var options = CreateOptions(testDatabase);
133+
134+
var threadCount = Environment.ProcessorCount;
135+
const int iterationsPerThread = 2;
136+
var clients = new ConcurrentBag<CosmosClient>();
137+
var tasks = new List<Task>();
138+
139+
for (var i = 0; i < threadCount; i++)
140+
{
141+
tasks.Add(Task.Run(async () =>
142+
{
143+
for (var j = 0; j < iterationsPerThread; j++)
144+
{
145+
await Task.Yield(); // Force context switching
146+
using var context = new CustomerContext(options);
147+
var client = context.Database.GetCosmosClient();
148+
clients.Add(client);
149+
}
150+
}));
151+
}
152+
153+
await Task.WhenAll(tasks);
154+
155+
// All retrieved clients should be the same instance
156+
var clientsArray = clients.ToArray();
157+
Assert.Equal(threadCount * iterationsPerThread, clientsArray.Length);
158+
159+
var uniqueClients = clientsArray.Distinct().ToArray();
160+
Assert.Single(uniqueClients); // Should only have one unique client instance
161+
}
162+
127163
private DbContextOptions CreateOptions(CosmosTestStore testDatabase, Action<DbContextOptionsBuilder> configure = null)
128164
{
129165
var builder = Fixture.AddOptions(testDatabase.AddProviderOptions(new DbContextOptionsBuilder()))

test/EFCore.Cosmos.FunctionalTests/ConnectionSpecificationTest.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,10 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
3636
public async Task Throws_for_missing_connection_info()
3737
{
3838
using var context = new NoConnectionContext();
39-
var creator = context.GetService<IDatabaseCreator>();
4039

4140
Assert.Equal(
4241
CosmosStrings.ConnectionInfoMissing,
43-
(await Assert.ThrowsAsync<InvalidOperationException>(() => creator.EnsureDeletedAsync())).Message);
42+
(await Assert.ThrowsAsync<InvalidOperationException>(() => context.GetService<IDatabaseCreator>().EnsureDeletedAsync())).Message);
4443
}
4544

4645
public class NoConnectionContext : DbContext

test/EFCore.Cosmos.Tests/Extensions/PartitionKeyBuilderExtensionsTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
120120
}
121121

122122
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
123-
=> optionsBuilder.UseCosmos("localhost", "_", "_");
123+
=> optionsBuilder.UseCosmos("https://localhost:8081", "C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==", "_");
124124
}
125125

126126
public class Customer1

0 commit comments

Comments
 (0)