Open davidfowl opened 2 years ago
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection See info in area-owners.md if you want to be subscribed.
DI council: @alexmg @tillig @pakrym @ENikS @ipjohnson @dadhi @seesharper @jeremydmiller @alistairjevans
FWIW, it feels weird to put IAsyncServiceProvider
in Microsoft.Extensions.DependencyInjection
when IServiceProvider
is in System
.
- There are no async constructors, we need to invent a constructor surrogate for this.
Related discussion, itself linking to this overarching issue.
Ok, so there's a few things to cover off here. Some of them may be specific to Autofac, some of them not.
We dipped our toe in the async-di water last year to add support for IAsyncDisposable
. This actually required relatively minimal changes to the overall Autofac codebase, primarily because disposal happens outside of the actual service resolve pipeline. At the time when we did that, I did consider what might be involved to go full-async, because honestly @davidfowl raising this proposal was just a matter of time.
I'll lead with what you describe as "implementation complexity", just because we should discuss the amount of change needed by us if you want to do async DI properly. Arguably, complexity isn't a superb reason to just not do something, but in this case...
Fundamentally, the entire Autofac resolve pipeline will need to be updated to use ValueTask<T>
instead of T
, and async/await as needed.
We can't only change the resolve path for the async registrations; the viral nature of async prevents it, unless we do sync-over-async when resolving those services. The ability to await a service resolve would have to be everywhere to actually gain the async benefits.
Attempting to ignore the viral nature of async by making assumptions in certain places has yielded a bug on
IAsyncDisposable
support already.
We also really don't want to maintain a sync and async resolve pipeline, that would be a huge amount of extra code maintenance, because, like I said, everything would have to change in our resolve pipeline.
So that all implies our pipeline becomes async by default, and we add a new ResolveAsync<TService>()
method on ILifetimeScope
, that becomes the "normal" resolve entry point. Then the existing sync Resolve<TService>()
method would invoke ResolveAsync<TService>()
, and if the returned ValueTask<TService>
did not complete synchronously we do what? Throw? It's effectively illegal to not await a ValueTask
you know is completing asynchronously, so not sure what we do there.
public class LifetimeScope
{
public T Resolve<T>()
{
var task = ResolveAsync<T>();
if (result.IsCompleted)
{
return task.Result;
}
// ??
throw new Exception("?");
}
public ValueTask<T> ResolveAsync()
{
// do the resolve
}
}
It's important to note that we cannot know before beginning the resolve whether or not the resolve operation will complete synchronously, so we basically have to "try" and somehow bail after the fact.
This all starts to get pretty complicated.
Users of Autofac often inject Lazy<T>
, which is based in the sync world.
Specifically, it has a property used to access the underlying service.
That won't work for async, so I imagine we would need a new LazyAsync<T>
? Would that live in System
as Lazy<T>
does?
Beyond technical problems, this puts an onus on the component to know which type of lazy it should use; this introduces coupling between the service and the dependency, which somewhat defies the point.
Autofac let's you inject invokable factories as Func<TService>
instances, to be resolved later. I imagine we would need to implement support for generating async versions of those factories.
One of the things we've generally asserted in Autofac is that a Resolve happens on a single thread. Because it's possible for users to create new lifetime scopes and generally do very weird stuff in resolve paths, there are locks in place that would need to be replaced, because the resolve would now be able to proceed on a different thread than the one it started on.
Most of our existing active integrations would need updating to understand/support async, which is quite an undertaking in and of itself.
There's almost certainly more challenges here than I've documented; these are just a few. I'm pretty confident that this would be a huge piece of work to implement for Autofac.
Beyond the actual difficulty though...
These changes could get more complex/confusing for users if an existing library component depends on a service that a user can override. For a random example, let's say a library Contoso.ConnectLib
has a dependency on Autofac, and uses our Lazy<T>
support to do lazy instantiation. (While it may not be best practice for public nuget libraries to take a direct dependency on a IoC container, this happens with internal packages all the time.)
Now, Contoso.ConnectLib
lets you override some behaviour by registering your own implementation of a service, and a component in the library injects that using Lazy<IMyService>
. Now, a user will look at Autofac and think, "cool, I can just register an async delegate for this type". Then they try to use the library and it explodes because you can't do a sync injection of the async type.
Either the library implementer has to change to support async DI injection, Autofac would have to change to have a compatibility "fallback" of doing sync-over-async, or there will be a lot of users having to apply workarounds like the below to get compatibility with their libraries.
// 🤮
builder.Register(ctx => new Lazy<MyService>(() => ctx.ResolveAsync<MyService>().Result));
I don't have the numbers on this, but I imagine the runtime team will; what are the overheads of a ValueTask
backed state machine being added to a method, even if the method completes synchronously? In the Autofac resolve pipeline, it would be sync in a huge percentage of cases. To do async resolve properly, a lot of methods inside Autofac would get async state machines, and I'm somewhat concerned about the extra CPU time spent doing all resolves, to support a small subset of dependencies doing async setup.
In 2020, Autofac v6 changed the way that Autofac can be extended by switching to a pipeline approach, and letting users add middleware. Middleware created since then may likely have to change to support users, even if they don't use async factory registration, which I can imagine might annoy our users somewhat.
As a parting note, I will add that Autofac is about object construction. That's pretty much it. People have tried to do some pretty crazy things with Autofac beyond that task, up to and including doing their entire app startup using build callbacks and the IStartable
interface. Adding async DI like this feels like it could make this worse.
I generally think that injecting a factory type, and then calling async methods on that factory when you need it, to get the appropriate instance, isn't the end of the world, and I'm not sure the trade-offs would be worth it.
I agree with @alistairjevans that the "native" async support will be a very complex task. So instead, I propose to make this an external extension to solve the specific problem.
If we concentrate only on the async resolution (because it is unclear to me how an async injection suppose to work), we may use much smaller API extensions of the DI libs to achieve the result.
Here is the working example from the DryIoc
using System;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using NUnit.Framework;
namespace DryIoc.Microsoft.DependencyInjection.Specification.Tests
{
public static class AsyncExt
{
public static IServiceCollection AddSingleton<TService>(this IServiceCollection services, Func<IServiceProvider, Task<TService>> asyncFactory)
{
var factoryID = Factory.GetNextID();
Task<TService> CreateServiceAsync(IServiceProvider sp)
{
var dryIoc = sp.GetRequiredService<IResolverContext>();
var result = dryIoc.SingletonScope.GetOrAddViaFactoryDelegate(factoryID, r => asyncFactory(r), dryIoc);
return (Task<TService>)result;
}
return services.AddSingleton<Func<IServiceProvider, Task<TService>>>(CreateServiceAsync);
}
public static Task<TService> GetRequiredServiceAsync<TService>(this IServiceProvider sp) =>
sp.GetRequiredService<Func<IServiceProvider, Task<TService>>>().Invoke(sp);
}
public class AsyncResolutionTestsPOC
{
[Test]
public async Task GetRequiredServiceAsync()
{
var services = new ServiceCollection();
services.AddSingleton<IRemoteConnectionFactory, TestConnectionFactory>();
services.AddSingleton<IRemoteConnection>(sp =>
{
var factory = sp.GetRequiredService<IRemoteConnectionFactory>();
return factory.ConnectAsync();
});
var providerFactory = new DryIocServiceProviderFactory();
var provider = providerFactory.CreateServiceProvider(providerFactory.CreateBuilder(services));
var connection1 = await provider.GetRequiredServiceAsync<IRemoteConnection>();
Assert.IsNotNull(connection1);
var connection2 = await provider.GetRequiredServiceAsync<IRemoteConnection>();
Assert.AreSame(connection2, connection1);
await connection2.PublishAsync("hello", "sailor");
}
public interface IRemoteConnection
{
Task PublishAsync(string channel, string message);
Task DisposeAsync();
}
public interface IRemoteConnectionFactory
{
Task<IRemoteConnection> ConnectAsync();
}
class TestConnectionFactory : IRemoteConnectionFactory
{
public Task<IRemoteConnection> ConnectAsync() => Task.FromResult<IRemoteConnection>(new TestRemoteConnection());
}
class TestRemoteConnection : IRemoteConnection
{
public Task DisposeAsync() => Task.CompletedTask;
public async Task PublishAsync(string channel, string message)
{
await Task.Delay(TimeSpan.FromMilliseconds(17));
Console.WriteLine(channel + "->" + message);
}
}
}
}
I went through implementation of async in one of my prototypes for Unity. Ended up with two independent pipelines with rather complex synchronization issues. For example, all thread based lifetimes stopped working in async.
After spending couple of months on this endeavor I decided to scratch support for async completely.
@alistairjevans Thanks for those thoughts. Those resonate and I understand how complex it would be to support this (as it forks the entire code base).
@dadhi I love this. I've listed it under alternative designs. There are some complexities around:
(because it is unclear to me how an async injection suppose to work),
I was thinking that we could look at an constructor surrogate. A static factory method on the type (Create or CreateAsync). You could even envision a static abstract interface method here (ICreateable<T, ...Tn>
). This could be added externally as well though, it could be an extension to ActivatorUtilitites
I've updated the API proposal. We need to agree on some semantics for how this extension would work. I like the idea that we would map async resolution to ValueTask/Task<TServiceType>
and we can build other conventions on top of those primitives.
@davidfowl
What happens when resolution fails. Is the Task cached?
In the demonstrated approach DryIoc will cache the task for the SingletonScope
or for the CurrentScope
lifetime. Moreover ValueTask
will be boxed to object.
But this is the current behavior and we may play with that.
Synchronizaton needs to be handled in this extension (if multiple people try to resolve the same Task, they should get the same instance).
Not sure. At least IScope.GetOrAddViaFactoryDelegate
ensures that the passed task-creation delegate will be called just once.
I could be missing something, but it seemed like the convenience of trying to wrap the async factory in the DI container was to abstract away the factory itself so you could "simply inject" the thing that required the async/await.
In the new proposal, it just moves the factory itself out a level, plus it introduces the need for any async resolution to use service location. How is that better than just resolving the factory and using it directly?
A bit nervous to be perhaps stating the obvious here but why not something like this:-
var services = new ServiceCollection();
services.AddSingleton<IRemoteConnectionFactory, RedisConnectionFactory>();
services.AddSingleton<Task<IRemoteConnection>>(sp =>
{
var factory = sp.GetRequiredService<IRemoteConnectionFactory>();
return factory.ConnectAsync();
});
ServiceProvider sp = services.BuildServiceProvider();
IRemoteConnection connection = await sp.GetRequiredServiceAsync<IRemoteConnection>();
public interface IRemoteConnection
{
Task PublishAsync(string channel, string message);
Task DisposeAsync();
}
public interface IRemoteConnectionFactory
{
Task<IRemoteConnection> ConnectAsync();
}
Here GetRequiredServiceAsync<IRemoteConnection>();
would act as more of a convenience method that resolves Task<IRemoteConnection>
and returns it.
Regardless of the above, some additional thoughts:-
Async calls are typically going to be more prone to transient or other exceptions, especially if they involve any network bound stuff.
I suspect the answers to the above would be that the containers wouldn't help here, and so that stuff would have to be "handled" by the developer in their underlying factory method implementations.. In other words, developer authored factory methods or classes would still be desired in perhaps most cases, if not initially, then eventually.
General observation: Placing async code in DI location takes control away from the application and hands it to the DI container. This is not necessarily wise without wrapping it with sufficient resiliency or other features and ensuring you have visibility and insight into failures and that the impact of a failure upon the application can be understood. By injecting a factory and calling an async method tylically within another async method the control flow is easy to follow and the impact easy to assess. By promoting the async factory method somewhere up into the container for DI to handle, I wonder whether that will actually make developers lives any easier when they do have to reason about failures or impacts. P.s I am not saying it will or it won't just a consideration.
I would expect that a failed async initialisation (e.g. database is down) would have the exception handled precisely the same way as throwing an exception in a constructor is now (e.g. an invalid requested state in the new object).
I'm not sure why it might want to be semantically distinct. It's still a failure to create the object you wanted.
If you require special handling for a transient error, you can do that in the async constructor or a try/catch, as usual.
On the subject of consuming services that might be created asynchronously...
I think that you want to be able to request both TService
and Task<TService>
for all services, regardless of how they're created/registered.
If your class doesn't do any async initialisation, you don't need to care. If you do, you can request a Task for everything (regardless of how it was created) and only await it when required.
That means when or if a dependency changes to async construction, your code doesn't need to change to take advantage of it.
For example, if you don't have any async initialisation in the second class, both classes have the same behaviour.
public class SyncConstructor(TService service) {}
public class AsyncConstructor{
private AsyncConstructor(TService service) {}
[AsyncConstructor] // or however we declare an async factory.
public static async Task<AsyncConstructor> Factory(Task<TService service>) {
// async stuff here...
return new AsyncConstructor(await service);
}
}
Of course, it only makes sense to wait for potentially async objects, but you could, for example, start requesting l10n from a slow resource, and it's suddenly an async service.
Background and motivation
There are cases where it is necessary to inject a dependency as a result of an asynchronous operation. This usually comes up when the dependency requires input from some IO operation (like retrieving a secret from a remote store). Today, developers tend to put blocking calls in factories:
The only other viable solution is to move that async operation to method calls, which results in deferring all IO until methods are called (where things can truly be async). This is a non-trivial refactoring that might be impossible depending on the circumstance. The idea here is to provide asynchronous construction support, so that these scenarios can work.
API Proposal
Async Resolution Support
Async Injection Support
These APIs would use the convention that async resolution is tied to
ValueTask/Task<TServiceType>
and would resolve the service and await the result as part of construction (see the example for more details).NOTE: The generic version could be done using static abstract interface methods and would be more trim friendly.
API Usage
Risks