OrchardCore
OrchardCore copied to clipboard
Difference in CreateChildContainer when cloning multiple of the same type vs one.
Describe the bug
When cloning services using CreateChildContainer we have a difference in behaviour between cloing multiple of the same service (see: https://github.com/OrchardCMS/OrchardCore/blob/a27a75eaa9fbd9e193569a42d4cdab9fcd28903e/src/OrchardCore/OrchardCore/Shell/Builders/Extensions/ServiceProviderExtensions.cs#L89) to if we were cloning just the one (see: https://github.com/OrchardCMS/OrchardCore/blob/a27a75eaa9fbd9e193569a42d4cdab9fcd28903e/src/OrchardCore/OrchardCore/Shell/Builders/Extensions/ServiceProviderExtensions.cs#L60)
The big difference here is that one uses the implementation factory and the other doesnt.
Is there a reason we do this? and should it all be going through the same process regardless of if its one or not...
Nice to see @Jetski5822 shine again :)
Is this causing any issue, or does it surface some discrepancy?
After JT's passing, paging the only person who was there at the time: @sebastienros.
This line also looks very suspicious, the created scope is never used:
https://github.com/OrchardCMS/OrchardCore/blob/a27a75eaa9fbd9e193569a42d4cdab9fcd28903e/src/OrchardCore/OrchardCore/Shell/Builders/Extensions/ServiceProviderExtensions.cs#L112
Because I was working on the shell container factory recently, I took a quick look. If I understand this code correctly, it is an optimization if there is a single service only. In that case, if the result is not disposable, creating the singletons can be deferred until a tenant resolves them. Otherwise all services must be created immediately.
So I think this should be kept, but the code is hard to understand. Maybe we can simplify it little bit.
It seems that this issue didn't really move for quite a while despite us asking the author for further feedback. Is this something you'd like to revisit any time soon or should we close? Please reply.
Closing this issue because it didn't receive further feedback from the author for very long. If you think this is still relevant, feel free to reopen it with the requested details.