Skip to content

Commit

Permalink
Add examples with minimal API which is more common
Browse files Browse the repository at this point in the history
  • Loading branch information
joeldickson authored Oct 17, 2024
1 parent 3f7e260 commit 2f25999
Showing 1 changed file with 47 additions and 14 deletions.
61 changes: 47 additions & 14 deletions doc/AG0043.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
# AG0043: BuildServiceProvider should not be used in production code

## Problem Description

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.

## Rule Details

This rule raises an issue when `BuildServiceProvider()` is called on `IServiceCollection` instances.

### Noncompliant Code Example
### Noncompliant Code Examples

#### Traditional ASP.NET Core
```csharp
public void ConfigureServices(IServiceCollection services)
{
Expand All @@ -19,8 +18,27 @@ public void ConfigureServices(IServiceCollection services)
}
```

### Compliant Solution
#### Minimal API
```csharp
var builder = WebApplication.CreateBuilder(args);

builder.Services.AddTransient<IMyService, MyService>();

var app = builder.Build();

app.MapGet("/", () =>
{
var serviceProvider = app.Services.BuildServiceProvider(); // Noncompliant
var myService = serviceProvider.GetService<IMyService>();
return myService.GetMessage();
});

app.Run();
```

### Compliant Solutions

#### Traditional ASP.NET Core
```csharp
public class Startup
{
Expand All @@ -29,44 +47,59 @@ public class Startup
services.AddTransient<IMyService, MyService>();
// Let ASP.NET Core build the service provider
}

public void Configure(IApplicationBuilder app, IMyService myService)
{
// Services are injected by the framework
}
}
```

## Why is this an Issue?
#### Minimal API
```csharp
var builder = WebApplication.CreateBuilder(args);

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.
builder.Services.AddTransient<IMyService, MyService>();

2. **Performance Impact**: Creating new service providers is computationally expensive and can impact application performance.
var app = builder.Build();

app.MapGet("/", (IMyService myService) => myService.GetMessage());

app.Run();

// Service interfaces and implementations
public interface IMyService
{
string GetMessage();
}

public class MyService : IMyService
{
public string GetMessage() => "Hello from MyService!";
}
```

## Why is this an Issue?
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.
2. **Performance Impact**: Creating new service providers is computationally expensive and can impact application performance.
3. **Singleton Duplication**: Multiple service providers can result in multiple instances of services that should be singletons.

## Exceptions

`BuildServiceProvider()` may be acceptable in the following scenarios:

- Unit tests where a full DI container is needed
- Development-time configuration validation
- Tools and utilities that run outside the normal application lifecycle

## How to Fix It

1. In ASP.NET Core applications:
- Let the framework handle service provider creation
- Use constructor injection to obtain dependencies
- Use `IServiceScope` for creating scoped service providers when needed
- HttpContext and other services have method for `RequestServices.GetService<T>` to get scoped services

- `HttpContext` and other services have methods like `RequestServices.GetService<T>()` to get scoped services
2. For configuration validation:
```csharp
public void ConfigureServices(IServiceCollection services)
{
services.AddTransient<IMyService, MyService>();

// Only in development
if (Environment.IsDevelopment())
{
Expand Down

0 comments on commit 2f25999

Please sign in to comment.