#98551 - Regression in DI scope validation#98661
Conversation
ghost
added
the
|
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection Issue DetailsFixes #98551
|
| // Store the result for each visited service | ||
| _scopedServices[callSite.Cache.Key] = scoped; | ||
| // If there is a scoped service in the call site tree, make sure we are not resolving it from a singleton | ||
| if (firstScopedServiceInCallSiteTree != null && argument.Singleton != null) |
| serviceCollection.AddSingleton<IFoo, Foo>(); | ||
|
|
||
| // Act + Assert | ||
| var aggregateException = Assert.Throws<AggregateException>(() => serviceCollection.BuildServiceProvider(new ServiceProviderOptions() { ValidateOnBuild = true, ValidateScopes = true })); |
There was a problem hiding this comment.
I assume without the fix here, that no exception was thrown here.
There was a problem hiding this comment.
An exception was indeed not thrown here, however it was thrown when registering the dependencies in the reversed order. The aspnet core repo happens to test the this scenario, while the runtime repo only covered the latter. This happened because in the previous code, the scoped-in-singleton check was not performed if a callsite was already cached. See here as well: #98551 (comment)
steveharter
left a comment
steveharter
left a comment
There was a problem hiding this comment.
LGTM; waiting a bit for others to comment.
|
We need this backported to release/9.0-preview2, it's blocking the build. @steveharter @benjaminpetit can you please validate the CI failure? |
|
The CI failure seems pre-existing: #98406 It's been signed-off by area owners so I'll go ahead and merge, then backport. |
|
@carlossanlop that's #98406. Build analysis is green. |
@steveharter please help keep an eye on this later. |
|
/backport to release/9.0-preview2 |
|
Started backporting to release/9.0-preview2: https://github.com/dotnet/runtime/actions/runs/8069902963 |
5 participants
Fixes #98551