Conversation
…disposable behavior Co-authored-by: rosebyte <14963300+rosebyte@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection |
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/AsyncServiceScope.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/AsyncServiceScope.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/AsyncServiceScope.cs
Outdated
Show resolved
Hide resolved
|
@copilot , I commented one file but the came comments apply to the other files too, fix them everywhere. |
Co-authored-by: rosebyte <14963300+rosebyte@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates API documentation in Microsoft.Extensions.DependencyInjection to make it explicit that synchronous Dispose() can throw when a scope/provider contains services that implement only IAsyncDisposable, and to recommend using DisposeAsync() / await using to ensure all disposables are handled.
Changes:
- Add explicit XML docs to
ServiceProvider.Dispose()/DisposeAsync()describing disposal behavior and the async-only disposable pitfall. - Add similar XML docs to
AsyncServiceScope.Dispose()/DisposeAsync(). - Extend
IServiceScoperemarks to call out the async-disposal recommendation and mentionAsyncServiceScope.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs | Replaces <inheritdoc /> with explicit docs describing sync vs async disposal behavior. |
| src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/IServiceScope.cs | Adds remarks warning about sync disposal when async-only disposables exist, and recommends async disposal patterns. |
| src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/AsyncServiceScope.cs | Documents the sync/async disposal behavior for the wrapper scope. |
src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/AsyncServiceScope.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/IServiceScope.cs
Outdated
Show resolved
Hide resolved
|
@copilot, I see the warning with IAsncDisposable instances throwing with Dispose is specific to ServiceProvider, remove changes in the other two files. |
Co-authored-by: rosebyte <14963300+rosebyte@users.noreply.github.com>
Done in e953d19 — reverted |
ServiceProvider.Dispose()throwsInvalidOperationExceptionwhen any resolved service implements onlyIAsyncDisposable. This behavior is undocumented, making it a surprising pitfall — especially when a new dependency introduces an async-only disposable service into an existing sync disposal flow.Changes
ServiceProvider.Dispose()— replaced/// <inheritdoc />with explicit docs noting it throwsInvalidOperationExceptionwhen an async-only service is resolved, and recommendingDisposeAsync()insteadServiceProvider.DisposeAsync()— documented as disposing all resolved services that implementIDisposableorIAsyncDisposableAsyncServiceScope.csandIServiceScope.csare left unchanged, as the throwing behavior is specific to theServiceProviderimplementation and is not part of the interface contract.Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.