Reduce chance of stopping a WebApplication incorrectly by providing compiler info/warning
- 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
Assessment
This issue has not been assessed yet.