Skip to content

Commit 57aeae5

Browse files
rannesjoeldickson
andauthored
Add BuildServiceProvider analyzer rule (#192)
* feat: add BuildServiceProvider analyzer rule * Update AG0041LogTemplateAnalyzer.cs (#193) * Update AG0041LogTemplateAnalyzer.cs * Create AG0041.md * feat: add BuildServiceProvider analyzer rule * docs: update docs for the rule. * chore: remove comments * Add examples with minimal API which is more common * Update error message with more explanation --------- Co-authored-by: Joel Dickson <[email protected]>
1 parent 44c34b4 commit 57aeae5

File tree

5 files changed

+321
-1
lines changed

5 files changed

+321
-1
lines changed

doc/AG0043.md

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
# AG0043: BuildServiceProvider should not be used in production code
2+
3+
## Problem Description
4+
Using `BuildServiceProvider()` in production code can lead to memory leaks and other issues because it creates a new container. This is especially problematic when called repeatedly, such as in request processing scenarios.
5+
6+
## Rule Details
7+
This rule raises an issue when `BuildServiceProvider()` is called on `IServiceCollection` instances.
8+
9+
### Noncompliant Code Examples
10+
11+
#### Traditional ASP.NET Core
12+
```csharp
13+
public void ConfigureServices(IServiceCollection services)
14+
{
15+
services.AddTransient<IMyService, MyService>();
16+
var serviceProvider = services.BuildServiceProvider(); // Noncompliant
17+
var myService = serviceProvider.GetService<IMyService>();
18+
}
19+
```
20+
21+
#### Minimal API
22+
```csharp
23+
var builder = WebApplication.CreateBuilder(args);
24+
25+
builder.Services.AddTransient<IMyService, MyService>();
26+
27+
var app = builder.Build();
28+
29+
app.MapGet("/", () =>
30+
{
31+
var serviceProvider = app.Services.BuildServiceProvider(); // Noncompliant
32+
var myService = serviceProvider.GetService<IMyService>();
33+
return myService.GetMessage();
34+
});
35+
36+
app.Run();
37+
```
38+
39+
### Compliant Solutions
40+
41+
#### Traditional ASP.NET Core
42+
```csharp
43+
public class Startup
44+
{
45+
public void ConfigureServices(IServiceCollection services)
46+
{
47+
services.AddTransient<IMyService, MyService>();
48+
// Let ASP.NET Core build the service provider
49+
}
50+
public void Configure(IApplicationBuilder app, IMyService myService)
51+
{
52+
// Services are injected by the framework
53+
}
54+
}
55+
```
56+
57+
#### Minimal API
58+
```csharp
59+
var builder = WebApplication.CreateBuilder(args);
60+
61+
builder.Services.AddTransient<IMyService, MyService>();
62+
63+
var app = builder.Build();
64+
65+
app.MapGet("/", (IMyService myService) => myService.GetMessage());
66+
67+
app.Run();
68+
69+
// Service interfaces and implementations
70+
public interface IMyService
71+
{
72+
string GetMessage();
73+
}
74+
75+
public class MyService : IMyService
76+
{
77+
public string GetMessage() => "Hello from MyService!";
78+
}
79+
```
80+
81+
## Why is this an Issue?
82+
1. **Memory Leaks**: Each call to `BuildServiceProvider()` creates a new dependency injection container, which holds references to all registered services. If called repeatedly (e.g., during request processing), this leads to memory leaks.
83+
2. **Performance Impact**: Creating new service providers is computationally expensive and can impact application performance.
84+
3. **Singleton Duplication**: Multiple service providers can result in multiple instances of services that should be singletons.
85+
86+
## Exceptions
87+
`BuildServiceProvider()` may be acceptable in the following scenarios:
88+
- Unit tests where a full DI container is needed
89+
- Development-time configuration validation
90+
- Tools and utilities that run outside the normal application lifecycle
91+
92+
## How to Fix It
93+
1. In ASP.NET Core applications:
94+
- Let the framework handle service provider creation
95+
- Use constructor injection to obtain dependencies
96+
- Use `IServiceScope` for creating scoped service providers when needed
97+
- `HttpContext` and other services have methods like `RequestServices.GetService<T>()` to get scoped services
98+
2. For configuration validation:
99+
```csharp
100+
public void ConfigureServices(IServiceCollection services)
101+
{
102+
services.AddTransient<IMyService, MyService>();
103+
// Only in development
104+
if (Environment.IsDevelopment())
105+
{
106+
// Validate service registration
107+
var serviceProvider = services.BuildServiceProvider(validateScopes: true);
108+
serviceProvider.Dispose();
109+
}
110+
}
111+
```
112+
113+
## Benefits
114+
- Prevents memory leaks
115+
- Improves application performance
116+
- Ensures proper singleton behavior
117+
- Maintains proper service scoping
118+
119+
## References
120+
- [ASP.NET Core Dependency Injection Best Practices](https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection#service-lifetimes)
121+
- [Dependency injection in ASP.NET Core](https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection)
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
using System.Threading.Tasks;
2+
using Agoda.Analyzers.AgodaCustom;
3+
using Agoda.Analyzers.Test.Helpers;
4+
using Microsoft.CodeAnalysis.Diagnostics;
5+
using NUnit.Framework;
6+
using Microsoft.Extensions.DependencyInjection;
7+
8+
namespace Agoda.Analyzers.Test.AgodaCustom;
9+
10+
[TestFixture]
11+
class AG0043UnitTests : DiagnosticVerifier
12+
{
13+
protected override DiagnosticAnalyzer DiagnosticAnalyzer => new AG0043NoBuildServiceProvider();
14+
15+
protected override string DiagnosticId => AG0043NoBuildServiceProvider.DIAGNOSTIC_ID;
16+
17+
[Test]
18+
public async Task BuildServiceProvider_Used_RaisesDiagnostic()
19+
{
20+
var services = new ServiceCollection();
21+
services.BuildServiceProvider();
22+
23+
var code = new CodeDescriptor
24+
{
25+
References = new[]
26+
{
27+
typeof(ServiceCollection).Assembly,
28+
typeof(ServiceProvider).Assembly,
29+
typeof(IServiceCollection).Assembly
30+
},
31+
Code = @"
32+
using Microsoft.Extensions.DependencyInjection;
33+
34+
public class Program
35+
{
36+
public static void Main()
37+
{
38+
var services = new ServiceCollection();
39+
var provider = services.BuildServiceProvider();
40+
}
41+
}"
42+
};
43+
44+
await VerifyDiagnosticsAsync(code, new DiagnosticLocation(9, 53));
45+
}
46+
47+
[Test]
48+
public async Task OtherMethod_NoDiagnostic()
49+
{
50+
var code = new CodeDescriptor
51+
{
52+
References = new[] {typeof(ServiceCollection).Assembly},
53+
Code = @"
54+
using Microsoft.Extensions.DependencyInjection;
55+
56+
class TestClass
57+
{
58+
public void ConfigureServices()
59+
{
60+
var services = new ServiceCollection();
61+
services.AddSingleton<object>();
62+
}
63+
}"
64+
};
65+
66+
await VerifyDiagnosticsAsync(code, EmptyDiagnosticResults);
67+
}
68+
69+
[Test]
70+
public async Task BuildServiceProvider_WhenChaining_RaiseDiagnostic()
71+
{
72+
var code = new CodeDescriptor
73+
{
74+
References = new[]
75+
{
76+
typeof(ServiceCollection).Assembly,
77+
typeof(ServiceProvider).Assembly,
78+
typeof(IServiceCollection).Assembly
79+
},
80+
Code = @"
81+
using Microsoft.Extensions.DependencyInjection;
82+
83+
public class Program
84+
{
85+
public static void Main()
86+
{
87+
var provider = new ServiceCollection()
88+
.AddSingleton<object>()
89+
.BuildServiceProvider();
90+
}
91+
}"
92+
};
93+
94+
await VerifyDiagnosticsAsync(code, new DiagnosticLocation(10, 34));
95+
}
96+
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
using System.Collections.Immutable;
2+
using System.Linq;
3+
using Agoda.Analyzers.Helpers;
4+
using Microsoft.CodeAnalysis;
5+
using Microsoft.CodeAnalysis.CSharp;
6+
using Microsoft.CodeAnalysis.CSharp.Syntax;
7+
using Microsoft.CodeAnalysis.Diagnostics;
8+
9+
namespace Agoda.Analyzers.AgodaCustom
10+
{
11+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
12+
public class AG0043NoBuildServiceProvider : DiagnosticAnalyzer
13+
{
14+
public const string DIAGNOSTIC_ID = "AG0043";
15+
16+
private static readonly LocalizableString Title = new LocalizableResourceString(
17+
nameof(CustomRulesResources.AG0043Title),
18+
CustomRulesResources.ResourceManager,
19+
typeof(CustomRulesResources));
20+
21+
private static readonly LocalizableString MessageFormat = new LocalizableResourceString(
22+
nameof(CustomRulesResources.AG0043Title),
23+
CustomRulesResources.ResourceManager,
24+
typeof(CustomRulesResources));
25+
26+
private static readonly LocalizableString Description
27+
= DescriptionContentLoader.GetAnalyzerDescription(nameof(AG0043NoBuildServiceProvider));
28+
29+
private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(
30+
DIAGNOSTIC_ID,
31+
Title,
32+
MessageFormat,
33+
AnalyzerCategory.CustomQualityRules,
34+
DiagnosticSeverity.Error,
35+
AnalyzerConstants.EnabledByDefault,
36+
Description,
37+
"https://github.com/agoda-com/AgodaAnalyzers/blob/master/doc/AG0043.md",
38+
WellKnownDiagnosticTags.EditAndContinue);
39+
40+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Descriptor);
41+
42+
public override void Initialize(AnalysisContext context)
43+
{
44+
context.EnableConcurrentExecution();
45+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
46+
context.RegisterSyntaxNodeAction(AnalyzeNode, SyntaxKind.InvocationExpression);
47+
}
48+
49+
private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
50+
{
51+
var invocationExpr = (InvocationExpressionSyntax)context.Node;
52+
53+
if (!(invocationExpr.Expression is MemberAccessExpressionSyntax memberAccessExpr))
54+
return;
55+
56+
if (memberAccessExpr.Name.Identifier.ValueText != "BuildServiceProvider")
57+
return;
58+
59+
var methodSymbol = context.SemanticModel.GetSymbolInfo(invocationExpr).Symbol as IMethodSymbol;
60+
if (methodSymbol == null)
61+
return;
62+
63+
if (!methodSymbol.ContainingNamespace.ToDisplayString()
64+
.StartsWith("Microsoft.Extensions.DependencyInjection"))
65+
return;
66+
67+
var containingType = methodSymbol.ContainingType;
68+
if (containingType == null)
69+
return;
70+
71+
var isServiceCollectionExtension = containingType
72+
.GetTypeMembers()
73+
.SelectMany(t => t.GetMembers())
74+
.OfType<IMethodSymbol>()
75+
.Any(m => m.Parameters.Length > 0 &&
76+
m.Parameters[0].Type.ToDisplayString() ==
77+
"Microsoft.Extensions.DependencyInjection.IServiceCollection");
78+
79+
var expressionType = context.SemanticModel.GetTypeInfo(memberAccessExpr.Expression).Type;
80+
var isExpressionServiceCollection = expressionType != null &&
81+
(expressionType.AllInterfaces.Any(i =>
82+
i.ToDisplayString() ==
83+
"Microsoft.Extensions.DependencyInjection.IServiceCollection") ||
84+
expressionType.ToDisplayString() ==
85+
"Microsoft.Extensions.DependencyInjection.IServiceCollection");
86+
87+
if (isServiceCollectionExtension || isExpressionServiceCollection)
88+
{
89+
var diagnostic = Diagnostic.Create(Descriptor, memberAccessExpr.Name.GetLocation());
90+
context.ReportDiagnostic(diagnostic);
91+
}
92+
}
93+
}
94+
}

src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.Designer.cs

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Agoda.Analyzers/AgodaCustom/CustomRulesResources.resx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,4 +260,7 @@ One exception is logging, where it can be useful to see the exact DC / cluster /
260260
<data name="AG0041Title" xml:space="preserve">
261261
<value>You are using either an interpolated string or string concatenation in your logs, change these to the message template format to preserve structure in your logs</value>
262262
</data>
263-
</root>
263+
<data name="AG0043Title" xml:space="preserve">
264+
<value>BuildServiceProvider detected in request processing code. This may cause memory leaks. Remove the BuildServiceProvider() call and let the framework manage the service provider lifecycle.</value>
265+
</data>
266+
</root>

0 commit comments

Comments
 (0)