DI Scope Should Dispose All Disposables On Dispose() #123620
Description
When Dispose() is invoked on ServiceProviderEngineScope, the engine correctly disposes all members implementing IDisposable. However, if any member implements IAsyncDisposable but not IDisposable, an exception is thrown, effectively forcing consumers to call DisposeAsync() instead.
I presume the motivation here was well‑intentioned, encouraging callers to prefer DisposeAsync(), thereby avoiding sync‑over‑async execution paths, which are generally undesirable. Since DisposeAsync() returns a ValueTask, it can indeed be regarded as the superior mechanism, however, Dispose() is not marked as deprecated, discouraged, or otherwise inferior, and there is no warning indicating that invoking it may result in an exception, as highlighted by @CarnaViire in this comment. The current behaviour risks surprising consumers and may obstruct their workflow, especially when they do not explicitly control the disposal flow.
Consider a system that has historically relied solely on IDisposable services. Introducing a new library that happens to register an IAsyncDisposable component would suddenly produce exceptions out of the blue, effectively compelling system owners to perform a broader refactor simply to adopt the library.
Similarly, if a library calls Dispose() on a scope rather than DisposeAsync(), its consumers would be unable to use any library that registers a type implementing only IAsyncDisposable. This is problematic because implementing only IAsyncDisposable is entirely legitimate, and the official guidance explicitly states that a type may implement both interfaces only when relevant to handling both synchronous and asynchronous disposal flows, not as a general requirement.
I noticed a comment on this behaviour in the original PR. However, the resolution appeared to focus primarily on the breaking‑change aspect, rather than evaluating the broader behavioural correctness of the approach.
@davidfowl, @halter73, I noticed you both contributed to the earlier discussion, and I’m curious what your view is on the behaviour described above. Should Dispose() ultimately support a sync‑over‑async path, or should we at least add a clear warning to the documentation to make this expectation explicit, if the current design is intentional?