Skip to content

Commit f0c5d1f

Browse files
ajcvickersajcvickers
committed
Start throwing by default for excessive service provider use
Issue #10236 Most of the changes here are two our tests where we use the internal service provider both as a convenience and to test specific behavior around the caching. The tests have been either: * Updated to use an external service provider explicitly * Updated to make use of a new option `ServiceProviderCachingEnabled`, which is on by default, but can be switched off to still use the internal service provider, but not to cache it. * Left untouched where the behavior being tested depends on service provider caching. This means our tests will not hit the error. We have a lot of testing of the general warning-as-error mechanism. Tested the new default throw here manually.
1 parent b5164b1 commit f0c5d1f

File tree

87 files changed

+904
-440
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

87 files changed

+904
-440
lines changed

‎src/EFCore.InMemory/Metadata/Conventions/Internal/InMemoryConventionSetBuilder.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ public static ConventionSet Build()
2828
{
2929
var serviceProvider = new ServiceCollection()
3030
.AddEntityFrameworkInMemoryDatabase()
31-
.AddDbContext<DbContext>(o => o.UseInMemoryDatabase(Guid.NewGuid().ToString()))
31+
.AddDbContext<DbContext>((p, o) =>
32+
o.UseInMemoryDatabase(Guid.NewGuid().ToString())
33+
.UseInternalServiceProvider(p))
3234
.BuildServiceProvider();
3335

3436
using (var serviceScope = serviceProvider.GetRequiredService<IServiceScopeFactory>().CreateScope())

‎src/EFCore.SqlServer/Metadata/Conventions/SqlServerConventionSetBuilder.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ public static ConventionSet Build()
8989
{
9090
var serviceProvider = new ServiceCollection()
9191
.AddEntityFrameworkSqlServer()
92-
.AddDbContext<DbContext>(o => o.UseSqlServer("Server=."))
92+
.AddDbContext<DbContext>((p, o) =>
93+
o.UseSqlServer("Server=.")
94+
.UseInternalServiceProvider(p))
9395
.BuildServiceProvider();
9496

9597
using (var serviceScope = serviceProvider.GetRequiredService<IServiceScopeFactory>().CreateScope())

‎src/EFCore.Sqlite.Core/Metadata/Conventions/SqliteConventionSetBuilder.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ public static ConventionSet Build()
3030
{
3131
var serviceProvider = new ServiceCollection()
3232
.AddEntityFrameworkSqlite()
33-
.AddDbContext<DbContext>(o => o.UseSqlite("Filename=_.db"))
33+
.AddDbContext<DbContext>((p, o) =>
34+
o.UseSqlite("Filename=_.db")
35+
.UseInternalServiceProvider(p))
3436
.BuildServiceProvider();
3537

3638
using (var serviceScope = serviceProvider.GetRequiredService<IServiceScopeFactory>().CreateScope())

‎src/EFCore/DbContextOptionsBuilder.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,27 @@ public virtual DbContextOptionsBuilder UseApplicationServiceProvider([CanBeNull]
199199
/// so that EF will manage the service providers and can create new instances as required.
200200
/// </para>
201201
/// </summary>
202+
/// <param name="sensitiveDataLoggingEnabled"> If <c>true</c>, then sensitive data is logged. </param>
202203
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
203204
public virtual DbContextOptionsBuilder EnableSensitiveDataLogging(bool sensitiveDataLoggingEnabled = true)
204205
=> WithOption(e => e.WithSensitiveDataLoggingEnabled(sensitiveDataLoggingEnabled));
205206

207+
/// <summary>
208+
/// <para>
209+
/// Enables or disables caching of internal service providers. Disabling caching can
210+
/// massively impact performance and should only be used in testing scenarios that
211+
/// build many service providers for test isolation.
212+
/// </para>
213+
/// <para>
214+
/// Note that if the application is setting the internal service provider through a call to
215+
/// <see cref="UseInternalServiceProvider" />, then setting this option wil have no effect.
216+
/// </para>
217+
/// </summary>
218+
/// <param name="cacheServiceProvider"> If <c>true</c>, then the internal service provider is cached. </param>
219+
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
220+
public virtual DbContextOptionsBuilder EnableServiceProviderCaching(bool cacheServiceProvider = true)
221+
=> WithOption(e => e.WithCacheServiceProvider(cacheServiceProvider));
222+
206223
/// <summary>
207224
/// <para>
208225
/// Sets the tracking behavior for LINQ queries run against the context. Disabling change tracking

‎src/EFCore/DbContextOptionsBuilder`.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,22 @@ public DbContextOptionsBuilder([NotNull] DbContextOptions<TContext> options)
185185
public new virtual DbContextOptionsBuilder<TContext> EnableSensitiveDataLogging(bool sensitiveDataLoggingEnabled = true)
186186
=> (DbContextOptionsBuilder<TContext>)base.EnableSensitiveDataLogging(sensitiveDataLoggingEnabled);
187187

188+
/// <summary>
189+
/// <para>
190+
/// Enables or disables caching of internal service providers. Disabling caching can
191+
/// massively impact performance and should only be used in testing scenarios that
192+
/// build many service providers for test isolation.
193+
/// </para>
194+
/// <para>
195+
/// Note that if the application is setting the internal service provider through a call to
196+
/// <see cref="UseInternalServiceProvider" />, then setting this option wil have no effect.
197+
/// </para>
198+
/// </summary>
199+
/// <param name="cacheServiceProvider"> If <c>true</c>, then the internal service provider is cached. </param>
200+
/// <returns> The same builder instance so that multiple calls can be chained. </returns>
201+
public new virtual DbContextOptionsBuilder<TContext> EnableServiceProviderCaching(bool cacheServiceProvider = true)
202+
=> (DbContextOptionsBuilder<TContext>)base.EnableServiceProviderCaching(cacheServiceProvider);
203+
188204
/// <summary>
189205
/// <para>
190206
/// Sets the tracking behavior for LINQ queries run against the context. Disabling change tracking

‎src/EFCore/Infrastructure/CoreOptionsExtension.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,11 @@ public class CoreOptionsExtension : IDbContextOptionsExtension
4141
private int? _maxPoolSize;
4242
private long? _serviceProviderHash;
4343
private string _logFragment;
44+
private bool _cacheServiceProvider = true;
4445

4546
private WarningsConfiguration _warningsConfiguration
4647
= new WarningsConfiguration()
48+
.TryWithExplicit(CoreEventId.ManyServiceProvidersCreatedWarning, WarningBehavior.Throw)
4749
.TryWithExplicit(CoreEventId.LazyLoadOnDisposedContextWarning, WarningBehavior.Throw)
4850
.TryWithExplicit(CoreEventId.DetachedLazyLoadingWarning, WarningBehavior.Throw);
4951

@@ -70,6 +72,7 @@ protected CoreOptionsExtension([NotNull] CoreOptionsExtension copyFrom)
7072
_warningsConfiguration = copyFrom.WarningsConfiguration;
7173
_queryTrackingBehavior = copyFrom.QueryTrackingBehavior;
7274
_maxPoolSize = copyFrom.MaxPoolSize;
75+
_cacheServiceProvider = copyFrom.ServiceProviderCachingEnabled;
7376

7477
if (copyFrom._replacedServices != null)
7578
{
@@ -254,6 +257,21 @@ public virtual CoreOptionsExtension WithWarningsConfiguration([CanBeNull] Warnin
254257
return clone;
255258
}
256259

260+
/// <summary>
261+
/// Creates a new instance with all options the same as for this instance, but with the given option changed.
262+
/// It is unusual to call this method directly. Instead use <see cref="DbContextOptionsBuilder" />.
263+
/// </summary>
264+
/// <param name="cacheServiceProvider"> The option to change. </param>
265+
/// <returns> A new instance with the option changed. </returns>
266+
public virtual CoreOptionsExtension WithCacheServiceProvider(bool cacheServiceProvider)
267+
{
268+
var clone = Clone();
269+
270+
clone._cacheServiceProvider = cacheServiceProvider;
271+
272+
return clone;
273+
}
274+
257275
/// <summary>
258276
/// The option set from the <see cref="DbContextOptionsBuilder.EnableSensitiveDataLogging" /> method.
259277
/// </summary>
@@ -299,6 +317,11 @@ public virtual CoreOptionsExtension WithWarningsConfiguration([CanBeNull] Warnin
299317
/// </summary>
300318
public virtual QueryTrackingBehavior QueryTrackingBehavior => _queryTrackingBehavior;
301319

320+
/// <summary>
321+
/// The option set from the <see cref="DbContextOptionsBuilder.EnableServiceProviderCaching" /> method.
322+
/// </summary>
323+
public virtual bool ServiceProviderCachingEnabled => _cacheServiceProvider;
324+
302325
/// <summary>
303326
/// The options set from the <see cref="DbContextOptionsBuilder.ReplaceService{TService,TImplementation}" /> method.
304327
/// </summary>

‎src/EFCore/Internal/ServiceProviderCache.cs‎

Lines changed: 62 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ public class ServiceProviderCache
3333
/// </summary>
3434
public virtual IServiceProvider GetOrAdd([NotNull] IDbContextOptions options, bool providerRequired)
3535
{
36-
var internalServiceProvider = options.FindExtension<CoreOptionsExtension>()?.InternalServiceProvider;
36+
var coreOptionsExtension = options.FindExtension<CoreOptionsExtension>();
37+
var internalServiceProvider = coreOptionsExtension?.InternalServiceProvider;
3738
if (internalServiceProvider != null)
3839
{
3940
ValidateOptions(options);
@@ -52,78 +53,83 @@ public virtual IServiceProvider GetOrAdd([NotNull] IDbContextOptions options, bo
5253
return internalServiceProvider;
5354
}
5455

55-
var key = options.Extensions
56-
.OrderBy(e => e.GetType().Name)
57-
.Aggregate(0L, (t, e) => (t * 397) ^ ((long)e.GetType().GetHashCode() * 397) ^ e.GetServiceProviderHashCode());
56+
(IServiceProvider ServiceProvider, IDictionary<string, string> DebugInfo) BuildServiceProvider()
57+
{
58+
ValidateOptions(options);
5859

59-
return _configurations.GetOrAdd(
60-
key,
61-
k =>
60+
var debugInfo = new Dictionary<string, string>();
61+
foreach (var optionsExtension in options.Extensions)
6262
{
63-
ValidateOptions(options);
64-
65-
var debugInfo = new Dictionary<string, string>();
66-
foreach (var optionsExtension in options.Extensions)
67-
{
68-
optionsExtension.PopulateDebugInfo(debugInfo);
69-
}
63+
optionsExtension.PopulateDebugInfo(debugInfo);
64+
}
7065

71-
debugInfo = debugInfo.OrderBy(v => debugInfo.Keys).ToDictionary(d => d.Key, v => v.Value);
66+
debugInfo = debugInfo.OrderBy(v => debugInfo.Keys).ToDictionary(d => d.Key, v => v.Value);
7267

73-
var services = new ServiceCollection();
74-
var hasProvider = ApplyServices(options, services);
68+
var services = new ServiceCollection();
69+
var hasProvider = ApplyServices(options, services);
7570

76-
var replacedServices = options.FindExtension<CoreOptionsExtension>()?.ReplacedServices;
77-
if (replacedServices != null)
71+
var replacedServices = coreOptionsExtension?.ReplacedServices;
72+
if (replacedServices != null)
73+
{
74+
// For replaced services we use the service collection to obtain the lifetime of
75+
// the service to replace. The replaced services are added to a new collection, after
76+
// which provider and core services are applied. This ensures that any patching happens
77+
// to the replaced service.
78+
var updatedServices = new ServiceCollection();
79+
foreach (var descriptor in services)
7880
{
79-
// For replaced services we use the service collection to obtain the lifetime of
80-
// the service to replace. The replaced services are added to a new collection, after
81-
// which provider and core services are applied. This ensures that any patching happens
82-
// to the replaced service.
83-
var updatedServices = new ServiceCollection();
84-
foreach (var descriptor in services)
81+
if (replacedServices.TryGetValue(descriptor.ServiceType, out var replacementType))
8582
{
86-
if (replacedServices.TryGetValue(descriptor.ServiceType, out var replacementType))
87-
{
88-
((IList<ServiceDescriptor>)updatedServices).Add(
89-
new ServiceDescriptor(descriptor.ServiceType, replacementType, descriptor.Lifetime));
90-
}
83+
((IList<ServiceDescriptor>) updatedServices).Add(
84+
new ServiceDescriptor(descriptor.ServiceType, replacementType, descriptor.Lifetime));
9185
}
92-
93-
ApplyServices(options, updatedServices);
94-
services = updatedServices;
9586
}
9687

97-
var serviceProvider = services.BuildServiceProvider();
88+
ApplyServices(options, updatedServices);
89+
services = updatedServices;
90+
}
9891

99-
if (hasProvider)
100-
{
101-
serviceProvider
102-
.GetRequiredService<ISingletonOptionsInitializer>()
103-
.EnsureInitialized(serviceProvider, options);
104-
}
92+
var serviceProvider = services.BuildServiceProvider();
93+
94+
if (hasProvider)
95+
{
96+
serviceProvider
97+
.GetRequiredService<ISingletonOptionsInitializer>()
98+
.EnsureInitialized(serviceProvider, options);
99+
}
105100

106-
var logger = serviceProvider.GetRequiredService<IDiagnosticsLogger<DbLoggerCategory.Infrastructure>>();
101+
var logger = serviceProvider.GetRequiredService<IDiagnosticsLogger<DbLoggerCategory.Infrastructure>>();
107102

108-
if (_configurations.Count == 0)
103+
if (_configurations.Count == 0)
104+
{
105+
logger.ServiceProviderCreated(serviceProvider);
106+
}
107+
else
108+
{
109+
logger.ServiceProviderDebugInfo(
110+
debugInfo,
111+
_configurations.Values.Select(v => v.DebugInfo).ToList());
112+
113+
if (_configurations.Count >= 20)
109114
{
110-
logger.ServiceProviderCreated(serviceProvider);
115+
logger.ManyServiceProvidersCreatedWarning(
116+
_configurations.Values.Select(e => e.ServiceProvider).ToList());
111117
}
112-
else
113-
{
114-
logger.ServiceProviderDebugInfo(
115-
debugInfo,
116-
_configurations.Values.Select(v => v.DebugInfo).ToList());
118+
}
117119

118-
if (_configurations.Count >= 20)
119-
{
120-
logger.ManyServiceProvidersCreatedWarning(
121-
_configurations.Values.Select(e => e.ServiceProvider).ToList());
122-
}
123-
}
120+
return (serviceProvider, debugInfo);
121+
}
122+
123+
if (coreOptionsExtension?.ServiceProviderCachingEnabled == false)
124+
{
125+
return BuildServiceProvider().ServiceProvider;
126+
}
127+
128+
var key = options.Extensions
129+
.OrderBy(e => e.GetType().Name)
130+
.Aggregate(0L, (t, e) => (t * 397) ^ ((long)e.GetType().GetHashCode() * 397) ^ e.GetServiceProviderHashCode());
124131

125-
return (serviceProvider, debugInfo);
126-
}).ServiceProvider;
132+
return _configurations.GetOrAdd(key, k => BuildServiceProvider()).ServiceProvider;
127133
}
128134

129135
private static void ValidateOptions(IDbContextOptions options)

‎test/EFCore.CrossStore.FunctionalTests/ConfigurationPatternsTest.cs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -243,13 +243,8 @@ public async Task Can_use_one_context_nested_inside_another_of_a_different_type(
243243
{
244244
using (SqlServerTestStore.GetNorthwindStore())
245245
{
246-
var inMemoryServiceProvider = new ServiceCollection()
247-
.AddEntityFrameworkInMemoryDatabase()
248-
.BuildServiceProvider();
249-
250-
var sqlServerServiceProvider = new ServiceCollection()
251-
.AddEntityFrameworkSqlServer()
252-
.BuildServiceProvider();
246+
var inMemoryServiceProvider = InMemoryFixture.DefaultServiceProvider;
247+
var sqlServerServiceProvider = SqlServerFixture.DefaultServiceProvider;
253248

254249
await NestedContextTest(
255250
() => new BlogContext(inMemoryServiceProvider),

‎test/EFCore.CrossStore.FunctionalTests/DiscriminatorTest.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ private class Context4285 : DbContext
106106
public DbSet<SubIntProduct2> SubIntProducts2 { get; set; }
107107

108108
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
109-
=> optionsBuilder.UseInMemoryDatabase(nameof(Context4285));
109+
=> optionsBuilder
110+
.UseInMemoryDatabase(nameof(Context4285));
110111

111112
protected override void OnModelCreating(ModelBuilder builder)
112113
{

‎test/EFCore.Design.Tests/Design/DbContextActivatorTest.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ public void CreateInstance_works()
1818
private class TestContext : DbContext
1919
{
2020
protected override void OnConfiguring(DbContextOptionsBuilder options)
21-
=> options.UseInMemoryDatabase(nameof(DbContextActivatorTest));
21+
=> options
22+
.EnableServiceProviderCaching(false)
23+
.UseInMemoryDatabase(nameof(DbContextActivatorTest));
2224
}
2325
}
2426
}

0 commit comments

Comments
 (0)