dotnet / dotnet/aspnetcore

Reduce chance of stopping a WebApplication incorrectly by providing compiler info/warning

Open
#50,578 1 comment 0 reactions 0 assignees View on GitHub
analyzer api-suggestion area-hosting enhancement
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

## Background and Motivation
There are two steps to shutting down a `WebApplication`, calling `StopAsync`, and disposing it. Omitting either of these steps can have negative consequences. If someone is transitioning from `WebHost` to `WebApplication`, the dispose behavior is different and can lead to introducing a bug/unwanted behavior. If you don't call `WebHost.StopAsync`, when you dispose the `WebHost` it implicitly calls `StopAsync`. `WebApplication` does not ensure `StopAsync` has been called when you dispose it. The consequences of this is that any `IHostedService` instances won't have their `StopAsync` method called when disposing a `WebApplication` whereas when using `WebHost` they did.
The second problem is if you omit disposing your `WebApplication`, you will leak some memory which is never reclaimed by GC. See #45621 for details.

## Proposed Analyzer

### Analyzer Behavior and Message
If a `WebApplication` is used with a using statement and there is not a call to StopAsync within the using statement code block, then an info message should be emitted informing the developer that any `IHostedService` instances may not be cleanly stopped if disposing without calling StopAsync. Similarly if not using a using block but manually calling `Dispose`/`DisposeAsync`, this is also the case. This would be an info usage rule.

If a call to `WebApplication.StopAsync` is made without a subsequent call to `WebApplication.Dispose()` or `WebApplication.DisposeAsync()`, then a warning should be emitted telling the developer that they will leak memory. Alternatively, fix issue #45621 and then this won't be needed, but there doesn't seem to be any traction on that issue. This would be a warning reliability rule.

### Category

- [ ] Design
- [ ] Documentation
- [ ] Globalization
- [ ] Interoperability
- [ ] Maintainability
- [ ] Naming
- [ ] Performance
- [x] Reliability
- [ ] Security
- [ ] Style
- [x] Usage

### Severity Level

- [ ] Error
- [x] Warning
- [x] Info
- [ ] Hidden

## Usage Scenarios
This should trigger the info about needing to call `StopAsync`:
```c#
var builder = WebApplication.CreateBuilder(Array.Empty());
builder.WebHost.UseKestrel(options =>
{
options.ListenLocalhost(8443);
});
builder.Logging.ClearProviders();
await using (var app = builder.Build())
{
app.Run(async context =>
{
await context.Response.WriteAsync("Hello world!");
});
await app.StartAsync();
await SomeExternalWaitingMechanismAsync();
}
```
This code should also trigger the info about needing to call `StopAsync`:
```c#
var builder = WebApplication.CreateBuilder(Array.Empty());
builder.WebHost.UseKestrel(options =>
{
options.ListenLocalhost(8443);
});
builder.Logging.ClearProviders();
var app = builder.Build();
app.Run(async context =>
{
await context.Response.WriteAsync("Hello world!");
});
await app.StartAsync();
await SomeExternalWaitingMechanismAsync();
```
Followed by one of the following lines:
```c#
((IDisposeable)app).Dispose(); // WebApplication explicitly implements IDisposable so must be cast
(app as IDisposable).Dispose();
(app as IDisposable)?.Dispose(); // null propagation as compiler can't tell whether app implements IDisposable
await app.DisposeAsync();
```

This should trigger the warning about needing to dispose the WebApplication
```c#
var builder = WebApplication.CreateBuilder(Array.Empty());
builder.WebHost.UseKestrel(options =>
{
options.ListenLocalhost(8443);
});
builder.Logging.ClearProviders();
var app = builder.Build();
app.Run(async context =>
{
await context.Response.WriteAsync("Hello world!");
});
await app.StartAsync();
await SomeExternalWaitingMechanismAsync();
await app.StopAsync();
```

## Risks
The overhead of running the analyzer will take a non-zero amount of time.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.