Property Injection with AllowCircularDependencies occurs too late; concurrency error

[alistairjevans]

In v6, this test fails:

[Fact]
public async Task WhenSeveralThreadsTryToAccessSharedPropertyInjectedInstance()
{
    for (var i = 0; i < 200; i++)
    {
        var builder = new ContainerBuilder();
        builder.RegisterType<A>();
        builder.RegisterType<B>().SingleInstance().PropertiesAutowired(PropertyWiringOptions.AllowCircularDependencies);
        var container = builder.Build();

        var t1 = Task.Run(() => container.Resolve<B>().AssertProp());
        var t2 = Task.Run(() => container.Resolve<B>().AssertProp());
        var t3 = Task.Run(() => container.Resolve<B>().AssertProp());
        var t4 = Task.Run(() => container.Resolve<B>().AssertProp());

        await Task.WhenAll(t1, t2, t3, t4);
    }
}

private sealed class B
{
    public A Prop { get; set; }

    public void AssertProp()
    {
        if (Prop is null)
        {
            throw new NullReferenceException();
        }
    }
}

This occurs because, in v6, property injection that permits circular dependency injection occurs in the RequestCompleting event, which fires at the end of the operation, after the synchronised shared instance activation; meaning that some of the requests will complete before property injection has finished.

This needs to be fixed, probably by moving the point in the pipeline that circular dependency detection happens to later, so we can more easily attach to before it.

[alistairjevans]

Looks like this isn't a new problem with v6; this same tests fails on 5.2.0. v6 replicated the same behaviour as v5.

I'm somewhat surprised people haven't noticed this before; perhaps Single Instances aren't that common for having PropertiesAutowired?

I'm going to look at fixing this, partly to stop the concurrency failure, and partly to fix the issue in DynamicProxy (#758).

[alistairjevans]

I'm actually not sure that there's a way to fix this...there's a contradiction at the heart of the problem:

1. For PropertyWiringOptions.AllowCircularDependencies to work, it has to run after all resolves have finished.
2. In order to inject properties in a thread-safe way for shared instances, we have to do Property Injection after the SharingMiddleware.

Those two requirements are mutually exclusive. Not sure of a way around it.

[tillig]

Just to capture a little more context here, a pipeline execution with property injection looks like this (based on the new DefaultDiagnosticTracer). In this case, it's a class that has a single string property being injected.

Resolve Operation Starting
{
  Resolve Request Starting
  {
    Service: Autofac.Specification.Test.Diagnostics.DefaultDiagnosticTracerTests+Implementor
    Component: Autofac.Specification.Test.Diagnostics.DefaultDiagnosticTracerTests+Implementor

    Pipeline:
    -> CircularDependencyDetectorMiddleware
      -> ScopeSelectionMiddleware
        -> SharingMiddleware
          -> RegistrationPipelineInvokeMiddleware
            -> ActivatorErrorHandlingMiddleware
              -> PropertiesAutowired
                -> DisposalTrackingMiddleware
                  -> Implementor (ReflectionActivator)
                  <- Implementor (ReflectionActivator)
                <- DisposalTrackingMiddleware
                Resolve Request Starting
                {
                  Service: System.String
                  Component: System.String

                  Pipeline:
                  -> CircularDependencyDetectorMiddleware
                    -> ScopeSelectionMiddleware
                      -> SharingMiddleware
                        -> RegistrationPipelineInvokeMiddleware
                          -> ActivatorErrorHandlingMiddleware
                            -> DisposalTrackingMiddleware
                              -> System.String
                              <- System.String
                            <- DisposalTrackingMiddleware
                          <- ActivatorErrorHandlingMiddleware
                        <- RegistrationPipelineInvokeMiddleware
                      <- SharingMiddleware
                    <- ScopeSelectionMiddleware
                  <- CircularDependencyDetectorMiddleware
                }
                Resolve Request Succeeded; result instance was HelloWorld
              <- PropertiesAutowired
            <- ActivatorErrorHandlingMiddleware
          <- RegistrationPipelineInvokeMiddleware
        <- SharingMiddleware
      <- ScopeSelectionMiddleware
    <- CircularDependencyDetectorMiddleware
  }
  Resolve Request Succeeded; result instance was Autofac.Specification.Test.Diagnostics.DefaultDiagnosticTracerTests+Implementor
}
Operation Succeeded; result instance was Autofac.Specification.Test.Diagnostics.DefaultDiagnosticTracerTests+Implementor

[alistairjevans]

The tricky thing is that the while the PropertiesAutowired middleware runs in the right place, it attaches to an event that fires way later, so the trace looks a bit like this in practice (see my comments with //):

Resolve Operation Starting
{
  Resolve Request Starting
  {
    Service: Autofac.Specification.Test.Diagnostics.DefaultDiagnosticTracerTests+Implementor
    Component: Autofac.Specification.Test.Diagnostics.DefaultDiagnosticTracerTests+Implementor

    Pipeline:
    -> CircularDependencyDetectorMiddleware
      -> ScopeSelectionMiddleware
        -> SharingMiddleware
          -> RegistrationPipelineInvokeMiddleware
            -> ActivatorErrorHandlingMiddleware
              -> PropertiesAutowired
                -> DisposalTrackingMiddleware
                  -> Implementor (ReflectionActivator)
                  <- Implementor (ReflectionActivator)
                <- DisposalTrackingMiddleware
                Resolve Request Starting
                {
                  Service: System.String
                  Component: System.String

                  Pipeline:
                  -> CircularDependencyDetectorMiddleware
                    -> ScopeSelectionMiddleware
                      -> SharingMiddleware
                        -> RegistrationPipelineInvokeMiddleware
                          -> ActivatorErrorHandlingMiddleware
                            -> DisposalTrackingMiddleware
                              -> System.String
                              <- System.String
                            <- DisposalTrackingMiddleware
                          <- ActivatorErrorHandlingMiddleware
                        <- RegistrationPipelineInvokeMiddleware
                      <- SharingMiddleware
                    <- ScopeSelectionMiddleware
                  <- CircularDependencyDetectorMiddleware
                }
                Resolve Request Succeeded; result instance was HelloWorld

              // Attach to RequestCompleting event....
              <- PropertiesAutowired

            <- ActivatorErrorHandlingMiddleware
          <- RegistrationPipelineInvokeMiddleware
        <- SharingMiddleware
      <- ScopeSelectionMiddleware
    <- CircularDependencyDetectorMiddleware
  }
  Resolve Request Succeeded; result instance was Autofac.Specification.Test.Diagnostics.DefaultDiagnosticTracerTests+Implementor
}

// ... RequestCompleting fires - injects properties onto instance.
Operation Succeeded; result instance was Autofac.Specification.Test.Diagnostics.DefaultDiagnosticTracerTests+Implementor

[tillig]

This issue has been open for quite some time without any traction. Much as we understand the challenges presented here, we also get very few substantial contributions and we don't have time to really dig into non-trivial issues like this. As such, we're closing this issue. If someone feels like they really want this, we would love to see a PR where this gets addressed.