Masstransit: Injecting IRequestClient in singleton services with M.E.DI

[poke]

Hey, I’ve noticed that the registration of request clients with Microsoft.Extensions.DependencyInjection looks like this:

https://github.com/MassTransit/MassTransit/blob/43561c1bcb09f620ccefb7e6ae4bc5208bbcdd8a/src/Containers/MassTransit.ExtensionsDependencyInjectionIntegration/Registration/DependencyInjectionContainerRegistrar.cs#L125-L143

So an IRequestClient<T> is registered both as a scoped and a singleton service. Note that this does not work with M.E.DI and only the latter registration is used when providing a service. So since the scoped registration comes last, every injection happens based on this registration.

This can become problematic when injecting the request client into singleton services because then this is basically violating the service scoping. If you have enabled validateScope when building the service provider this will cause a runtime exception:

InvalidOperationException: Error while validating the service descriptor 'ServiceType: ExampleProject.IExampleService Lifetime: Singleton ImplementationType: ExampleProject.ExampleService': Cannot consume scoped service 'MassTransit.IRequestClient`1[ExampleProject.ExampleRequestMessage]' from singleton 'ExampleProject.IExampleService'.

Unfortunately, there isn’t really a way to remedy this, apart from registering the singleton client using a different type but I’m not sure if that’s a nice solution here. I personally just inject the IClientFactory now and just create the client there.

[phatboyg]

Since when? I've tested it and it works both ways. Even in the unit tests.

[poke]

Oh, my mistake: validateScope: true only validates the scope when the service (in my case above IExampleService) is resolved. It is ServiceProviderOptions.ValidateOnBuild that performs this check when the service provider is being built.

See the following example (written in LINQPad, but should equally translate to a console application):

```c#
void Main()
{
var services = new ServiceCollection();
services.AddSingleton();
services.AddMassTransit(x =>
{
x.AddBus(provider => Bus.Factory.CreateUsingRabbitMq(cfg =>
{
cfg.Host("localhost");
cfg.ConfigureEndpoints(provider);
}));
x.AddRequestClient();
});

var sp = services.BuildServiceProvider(new ServiceProviderOptions { ValidateScopes = true, ValidateOnBuild = true });
var testService = sp.GetService<TestService>();

}

public class TestService
{
public TestService(IRequestClient client) { }
}
public interface ExampleMessage { }
```

With ValidateScopes = true and ValidateOnBuild = false, this only throws on the sp.GetService<TestService>() line. With both set to true, this already throws on BuildServiceProvider().

The ValidateOnBuild was added for 3.0.0 and has also been enabled by default for the generic host for 3.0.0.

The underlying behavior, that M.E.DI only resolves the latest registration, regardless of whether there would have been a “better” lifetime configuration, has been always the case I think.

Can you point me at the unit tests that should cover this so I can take a look if they are missing something?

[phatboyg]

May be related: https://github.com/MassTransit/MassTransit/issues/1581

[phatboyg]

This example resolves both clients, one scoped, one singleton:
https://github.com/MassTransit/MassTransit/blob/develop/src/Containers/MassTransit.Containers.Tests/DependencyInjection_Tests/DependencyInjection_RequestClient.cs

[poke]

But it doesn’t validate the scopes:

https://github.com/MassTransit/MassTransit/blob/43561c1bcb09f620ccefb7e6ae4bc5208bbcdd8a/src/Containers/MassTransit.Containers.Tests/DependencyInjection_Tests/DependencyInjection_RequestClient.cs#L43

So you are essentially ignoring all issues that may come from this which means that you are effectively lifting the scoped service into a singleton lifetime. This doesn’t appear to be problematic for the request client itself but may be for the ConsumeContext, the scoped instance keeps around.

If you set both ValidateScopes and ValidateOnBuild to false in my above example, then this will compile and “work” but you are still only hiding the problem, not solving it.

You can confirm this problem by two ways in the repo:

• Change the DependencyInjection_RequestClient.cs to build the service provider _with_ validated scopes. MT is depending on 2.2, so ValidateOnBuild is not yet available, but ValidateScopes will be enough to see the problem:

  c# _provider = collection.BuildServiceProvider(true);

  This causes a failure in DependencyInjection_RequestClient_Context.Should_receive_the_response because the validation prevents the test from retrieving a scoped service on the root provider.

• Or in DependencyInjectionContainerRegistrar.cs throw an exception at the beginning of the singleton registrations. If the code ever ran, it should cause some failure with the tests but it doesn’t:

  c# _collection.AddSingleton(provider => { throw new Exception("Singleton is never resolved"); // …

  But all tests pass here, so this means that the singleton registration is never used and instead it will always resolve the scoped registration.

[phatboyg]

The Singleton has to be used, otherwise the one expecting a ConsumeContext would throw an exception, since there isn't a ConsumeContext in the scope.

[poke]

GetConsumeContext resolves the context through the ScopedConsumeContextProvider:

https://github.com/MassTransit/MassTransit/blob/43561c1bcb09f620ccefb7e6ae4bc5208bbcdd8a/src/Containers/MassTransit.ExtensionsDependencyInjectionIntegration/DependencyInjectionExtensions.cs#L42-L45

ScopedConsumeContextProvider is registered as a scoped service itself, so there is the same problem as with the request client, so it gets incorrectly resolved and then returns a null context. And that is handled in the (scoped) factory method for the request client.

[phatboyg]

If what you're saying is true, I could remove the singleton registration and the tests should still pass?

[poke]

Yup, as I mentioned above, I threw an exception right at the beginning and no test failed:

diff --git a/src/Containers/MassTransit.ExtensionsDependencyInjectionIntegration/Registration/DependencyInjectionContainerRegistrar.cs b/src/Containers/MassTransit.ExtensionsDependencyInjectionIntegration/Registration/DependencyInjectionContainerRegistrar.cs
index 4ee4acebc..5d3738acf 100644
--- a/src/Containers/MassTransit.ExtensionsDependencyInjectionIntegration/Registration/DependencyInjectionContainerRegistrar.cs
+++ b/src/Containers/MassTransit.ExtensionsDependencyInjectionIntegration/Registration/DependencyInjectionContainerRegistrar.cs
@@ -106,6 +106,7 @@ namespace MassTransit.ExtensionsDependencyInjectionIntegration.Registration
         {
             _collection.AddSingleton(provider =>
             {
+                throw new Exception("Singleton is never resolved");
                 var clientFactory = provider.GetRequiredService<IClientFactory>();

                 return clientFactory.CreateRequestClient<T>(timeout);
@@ -126,6 +127,7 @@ namespace MassTransit.ExtensionsDependencyInjectionIntegration.Registration
         {
             _collection.AddSingleton(provider =>
             {
+                throw new Exception("Singleton is never resolved");
                 var clientFactory = provider.GetRequiredService<IClientFactory>();

                 return clientFactory.CreateRequestClient<T>(destinationAddress, timeout);

[phatboyg]

So, yeah, the singleton (non-scoped) version is going to have to be built on-demand using the IClientFactory as you are doing. I'm going to add a couple of extension methods to help out, but I don't see any other way around it.

[poke]

That change looks sensible. Too bad that there isn’t really another way here..

Thank you very much for getting behind this so quickly! :relaxed:

[phatboyg]

That change looks sensible. Too bad that there isn’t really another way here..

Yeah, I really wish there was a hybrid lifecycle for a single interface. But that's a limitation of MS DI, can't work around it. Thanks for your help tracking it down.

[Timovzl]

Just to make sure I'm drawing the right conclusions from this conversation and my own tests:

IClientFactory is a singleton. Its CreateRequestClient<T> is thread-safe, allowing the singleton to be shared by all threads.

The resulting clients are not thread-safe, nor do they need to be, because we create them on-the-fly. The clients can be used to perform the actual requests.

Is this accurate?

[phatboyg]

The resulting clients are, in fact, thread safe.