Skip to content

Commit

Permalink
Bug fix: calls to GetService and GetRegistration might return null ev…
Browse files Browse the repository at this point in the history
…en after type was registered. Fixes #909
  • Loading branch information
dotnetjunkie committed Jun 11, 2021
1 parent f1c8c9a commit 3ef3f9a
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
41 changes: 41 additions & 0 deletions src/SimpleInjector.Tests.Unit/GetRegistrationOfTTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -316,5 +316,46 @@ public void GetRegistration_OnInvalidButRegisteredType_ReturnsThatRegistration()
Assert.IsNotNull(registration, "The GetRegistration method is expected to return an " +
"InstanceProducer since it is explicitly registered by the user.");
}

// #909
[TestMethod]
public void GetRegistration_OnRegisteredTypeWithGetRegistrationCalledBeforeRegistration_ReturnsThatRegistration()
{
// Arrange
var container = new Container();

var registration = container.GetRegistration<RealTimeProvider>();

Assert.IsNull(registration, "Test setup failed.");

container.Register<RealTimeProvider>();

// Act
registration = container.GetRegistration<RealTimeProvider>();

// Assert
Assert.IsNotNull(registration);
}

// #909
[TestMethod]
public void GetService_OnRegisteredTypeWithGetRegistrationCalledBeforeRegistration_ReturnsThatService()
{
// Arrange
var container = new Container();
IServiceProvider provider = container;

var registration = container.GetRegistration<RealTimeProvider>();

Assert.IsNull(registration, "Test setup failed.");

container.Register<RealTimeProvider>();

// Act
var service = provider.GetService(typeof(RealTimeProvider));

// Assert
Assert.IsNotNull(service);
}
}
}
5 changes: 5 additions & 0 deletions src/SimpleInjector/Container.Common.cs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,11 @@ private void AddInstanceProducer(InstanceProducer producer)
entry.Add(producer);

this.RemoveExternalProducer(producer);

// There could already be a 'null' entry in the root-producer cache, which will happen when
// GetRegistration(X) is called before X is registered. We need to clear the cache to prevent
// the cached 'null' to be returned from GetRegistration instead of this producer. See #909.
this.rootProducerCache.Remove(producer.ServiceType);
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/SimpleInjector/Container.Resolving.cs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public IEnumerable<object> GetAllInstances(Type serviceType)
if (producer != null)
{
// The producer is created implicitly. This forces us to lock the container. Such
// implicit registration could be done through in numberous ways (e.g. through
// implicit registration could be done through in numerous ways (e.g. through
// unregistered type resolution, or because the type is concrete). Being able to make
// registrations after such call, could lead to unexpected behavior, which is why
// locking the container is important.
Expand All @@ -252,7 +252,10 @@ public IEnumerable<object> GetAllInstances(Type serviceType)
}
}

// Add the producer, even when it's null.
// PERF: Add the producer, even when it's null to prevent this if-block from re-executing when
// the method is called more often for the same service type. Performance of this if-block is
// slow and the users might call it many times in the happy path of their application. Also
// note that GetService is calling back into this method
this.AppendRootInstanceProducer(serviceType, producer);
}

Expand Down

0 comments on commit 3ef3f9a

Please sign in to comment.