Sort out the thread-safety

This commit is contained in:
Antony Male 2014-05-11 22:35:21 +01:00
parent b89c61a91e
commit af06ee5019
5 changed files with 114 additions and 87 deletions

View File

@ -4,14 +4,16 @@ using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
namespace StyletIoC
{
internal class BuilderUpper
{
private Type type;
private StyletIoCContainer container;
private readonly Type type;
private readonly StyletIoCContainer container;
private readonly object implementorLock = new object();
private Action<object> implementor;
public BuilderUpper(Type type, StyletIoCContainer container)
@ -50,15 +52,18 @@ namespace StyletIoC
public Action<object> GetImplementor()
{
if (this.implementor != null)
lock (this.implementorLock)
{
if (this.implementor != null)
return this.implementor;
var parameterExpression = Expression.Parameter(typeof(object), "inputParameter");
var typedParameterExpression = Expression.Convert(parameterExpression, this.type);
var expression = this.GetExpression(typedParameterExpression);
this.implementor = Expression.Lambda<Action<object>>(this.GetExpression(typedParameterExpression), parameterExpression).Compile();
return this.implementor;
var parameterExpression = Expression.Parameter(typeof(object), "inputParameter");
var typedParameterExpression = Expression.Convert(parameterExpression, this.type);
var expression = this.GetExpression(typedParameterExpression);
this.implementor = Expression.Lambda<Action<object>>(this.GetExpression(typedParameterExpression), parameterExpression).Compile();
return this.implementor;
}
}
}
}

View File

@ -4,6 +4,7 @@ using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
namespace StyletIoC
@ -52,7 +53,11 @@ namespace StyletIoC
// Sealed so Code Analysis doesn't moan about us setting the virtual Type property
internal sealed class TypeCreator : CreatorBase
{
public string AttributeKey { get; private set; }
private readonly string _attributeKey;
public string AttributeKey
{
get { return this._attributeKey; }
}
private Expression creationExpression;
public TypeCreator(Type type, StyletIoCContainer container) : base(container)
@ -62,7 +67,7 @@ namespace StyletIoC
// Use the key from InjectAttribute (if present), and let someone else override it if they want
var attribute = (InjectAttribute)type.GetCustomAttributes(typeof(InjectAttribute), false).FirstOrDefault();
if (attribute != null)
this.AttributeKey = attribute.Key;
this._attributeKey = attribute.Key;
}
private string KeyForParameter(ParameterInfo parameter)
@ -132,15 +137,15 @@ namespace StyletIoC
var completeExpression = this.CompleteExpressionFromCreator(creator);
this.creationExpression = completeExpression;
return completeExpression;
Interlocked.CompareExchange(ref this.creationExpression, completeExpression, null);
return this.creationExpression;
}
}
// Sealed for consistency with TypeCreator
internal sealed class FactoryCreator<T> : CreatorBase
{
private Func<StyletIoCContainer, T> factory;
private readonly Func<StyletIoCContainer, T> factory;
private Expression instanceExpression;
public override Type Type { get { return typeof(T); } }
@ -160,8 +165,8 @@ namespace StyletIoC
var completeExpression = this.CompleteExpressionFromCreator(invoked);
this.instanceExpression = completeExpression;
return completeExpression;
Interlocked.CompareExchange(ref this.instanceExpression, completeExpression, null);
return this.instanceExpression;
}
}
}

View File

@ -17,10 +17,10 @@ namespace StyletIoC
internal abstract class RegistrationBase : IRegistration
{
protected ICreator creator;
protected readonly ICreator creator;
public Type Type { get { return this.creator.Type; } }
protected readonly object generatorLock = new object();
// Value type, so needs locked access
protected Func<object> generator { get; set; }
public RegistrationBase(ICreator creator)
@ -44,9 +44,18 @@ namespace StyletIoC
public override Func<object> GetGenerator()
{
if (this.generator == null)
this.generator = Expression.Lambda<Func<object>>(this.GetInstanceExpression()).Compile();
return this.generator;
// Compiling the generator might be expensive, but there's nothing to be gained from
// doing it outside of the lock - the altnerative is having two threads compiling it in parallel,
// while would take just as long and use more resources
lock (this.generatorLock)
{
if (this.generator != null)
return this.generator;
var generator = Expression.Lambda<Func<object>>(this.GetInstanceExpression()).Compile();
if (this.generator == null)
this.generator = generator;
return this.generator;
}
}
}
@ -63,17 +72,21 @@ namespace StyletIoC
return;
// Ensure we don't end up creating two singletons, one used by each thread
Interlocked.CompareExchange(ref this.instance, Expression.Lambda<Func<object>>(this.creator.GetInstanceExpression()).Compile()(), null);
var instance = Expression.Lambda<Func<object>>(this.creator.GetInstanceExpression()).Compile()();
Interlocked.CompareExchange(ref this.instance, instance, null);
}
public override Func<object> GetGenerator()
{
this.EnsureInstantiated();
if (this.generator == null)
this.generator = () => this.instance;
return this.generator;
// Cheap delegate creation, so doesn't need to be outside the lock
lock (this.generatorLock)
{
if (this.generator == null)
this.generator = () => this.instance;
return this.generator;
}
}
public override Expression GetInstanceExpression()
@ -84,32 +97,41 @@ namespace StyletIoC
this.EnsureInstantiated();
// This expression yields the actual type of instance, not 'object'
this.instanceExpression = Expression.Constant(this.instance);
var instanceExpression = Expression.Constant(this.instance);
Interlocked.CompareExchange(ref this.instanceExpression, instanceExpression, null);
return this.instanceExpression;
}
}
internal class GetAllRegistration : IRegistration
{
private StyletIoCContainer container;
private readonly StyletIoCContainer container;
public string Key { get; set; }
public Type Type { get; private set; }
private readonly Type _type;
public Type Type
{
get { return this._type; }
}
private Expression expression;
private readonly object generatorLock = new object();
private Func<object> generator;
public GetAllRegistration(Type type, StyletIoCContainer container)
{
this.Type = type;
this._type = type;
this.container = container;
}
public Func<object> GetGenerator()
{
if (this.generator == null)
this.generator = Expression.Lambda<Func<object>>(this.GetInstanceExpression()).Compile();
return this.generator;
lock (this.generatorLock)
{
if (this.generator == null)
this.generator = Expression.Lambda<Func<object>>(this.GetInstanceExpression()).Compile();
return this.generator;
}
}
public Expression GetInstanceExpression()
@ -120,7 +142,7 @@ namespace StyletIoC
var list = Expression.New(this.Type);
var init = Expression.ListInit(list, this.container.GetRegistrations(new TypeKey(this.Type.GenericTypeArguments[0], this.Key), false).GetAll().Select(x => x.GetInstanceExpression()));
this.expression = init;
Interlocked.CompareExchange(ref this.expression, init, null);
return this.expression;
}
}

View File

@ -16,7 +16,7 @@ namespace StyletIoC
internal class SingleRegistration : IRegistrationCollection
{
private IRegistration registration;
private readonly IRegistration registration;
public SingleRegistration(IRegistration registration)
{
@ -43,7 +43,8 @@ namespace StyletIoC
internal class RegistrationCollection : IRegistrationCollection
{
private List<IRegistration> registrations;
private readonly object registrationsLock = new object();
private readonly List<IRegistration> registrations;
public RegistrationCollection(List<IRegistration> registrations)
{
@ -58,14 +59,14 @@ namespace StyletIoC
public List<IRegistration> GetAll()
{
List<IRegistration> registrationsCopy;
lock (this.registrations) { registrationsCopy = registrations.ToList(); }
lock (this.registrationsLock) { registrationsCopy = registrations.ToList(); }
return registrationsCopy;
}
public IRegistrationCollection AddRegistration(IRegistration registration)
{
// Need to lock the list, as someone might be fetching from it while we do this
lock (this.registrations)
lock (this.registrationsLock)
{
// Should have been caught by SingleRegistration.AddRegistration
Debug.Assert(!this.registrations.Any(x => x.Type == registration.Type));

View File

@ -100,9 +100,9 @@ namespace StyletIoC
/// <summary>
/// Maps a [type, key] pair, where 'type' is an unbound generic (something like IValidator{}) to something which, given a type, can create an IRegistration for that type.
/// So if they've bound an IValidator{} to a an IntValidator, StringValidator, etc, and request an IValidator{string}, one of the UnboundGenerics here can generatr a StringValidator.
/// Type-safety with the List is ensured by locking using the list as the lock object before modifying / iterating.
/// </summary>
private readonly ConcurrentDictionary<TypeKey, List<UnboundGeneric>> unboundGenerics = new ConcurrentDictionary<TypeKey, List<UnboundGeneric>>();
/// <remarks>Dictionary{TKey, TValue} and List{T} are thread-safe for concurrent reads, which is all that happens after building</remarks>
private readonly Dictionary<TypeKey, List<UnboundGeneric>> unboundGenerics = new Dictionary<TypeKey, List<UnboundGeneric>>();
/// <summary>
/// Maps a type onto a BuilderUpper for that type, which can create an Expresson/Delegate to build up that type.
@ -321,41 +321,37 @@ namespace StyletIoC
if (!this.unboundGenerics.TryGetValue(new TypeKey(unboundGenericType, typeKey.Key), out unboundGenerics))
return false;
// Need to lock this, as someone might modify the underying list by registering a new unbound generic
lock (unboundGenerics)
foreach (var unboundGeneric in unboundGenerics)
{
foreach (var unboundGeneric in unboundGenerics)
// Consider this scenario:
// interface IC<T, U> { } class C<T, U> : IC<U, T> { }
// Then they ask for an IC<int, bool>. We need to give them a C<bool, int>
// Search the ancestry of C for an IC (called implOfUnboundGenericType), then create a mapping which says that
// U is a bool and T is an int by comparing this against 'type' - the IC<T, U> that's registered as the service
// Then use this when making the type for C
Type newType;
if (unboundGeneric.Type == unboundGenericType)
{
// Consider this scenario:
// interface IC<T, U> { } class C<T, U> : IC<U, T> { }
// Then they ask for an IC<int, bool>. We need to give them a C<bool, int>
// Search the ancestry of C for an IC (called implOfUnboundGenericType), then create a mapping which says that
// U is a bool and T is an int by comparing this against 'type' - the IC<T, U> that's registered as the service
// Then use this when making the type for C
Type newType;
if (unboundGeneric.Type == unboundGenericType)
{
newType = type;
}
else
{
var implOfUnboundGenericType = unboundGeneric.Type.GetBaseTypesAndInterfaces().Single(x => x.Name == unboundGenericType.Name);
var mapping = implOfUnboundGenericType.GenericTypeArguments.Zip(type.GenericTypeArguments, (n, t) => new { Type = t, Name = n });
newType = unboundGeneric.Type.MakeGenericType(unboundGeneric.Type.GetTypeInfo().GenericTypeParameters.Select(x => mapping.Single(t => t.Name.Name == x.Name).Type).ToArray());
}
// The binder should have made sure of this
Debug.Assert(type.IsAssignableFrom(newType));
// Right! We've made a new generic type we can use
var registration = unboundGeneric.CreateRegistrationForType(newType);
// AddRegistration returns the IRegistrationCollection which was added/updated, so the one returned from the final
// call to AddRegistration is the final IRegistrationCollection for this key
registrations = this.AddRegistration(typeKey, registration);
newType = type;
}
else
{
var implOfUnboundGenericType = unboundGeneric.Type.GetBaseTypesAndInterfaces().Single(x => x.Name == unboundGenericType.Name);
var mapping = implOfUnboundGenericType.GenericTypeArguments.Zip(type.GenericTypeArguments, (n, t) => new { Type = t, Name = n });
newType = unboundGeneric.Type.MakeGenericType(unboundGeneric.Type.GetTypeInfo().GenericTypeParameters.Select(x => mapping.Single(t => t.Name.Name == x.Name).Type).ToArray());
}
// The binder should have made sure of this
Debug.Assert(type.IsAssignableFrom(newType));
// Right! We've made a new generic type we can use
var registration = unboundGeneric.CreateRegistrationForType(newType);
// AddRegistration returns the IRegistrationCollection which was added/updated, so the one returned from the final
// call to AddRegistration is the final IRegistrationCollection for this key
registrations = this.AddRegistration(typeKey, registration);
}
return registrations != null;
@ -408,25 +404,23 @@ namespace StyletIoC
internal void AddUnboundGeneric(TypeKey typeKey, UnboundGeneric unboundGeneric)
{
// We're not worried about thread-safety across multiple calls to this function (as it's only called as part of setup, which we're
// not thread-safe about). However someone might be fetching something from this list while we're modifying it, which we need to avoid
var unboundGenerics = this.unboundGenerics.GetOrAdd(typeKey, x => new List<UnboundGeneric>());
lock (unboundGenerics)
// not thread-safe about).
List<UnboundGeneric> unboundGenerics;
if (!this.unboundGenerics.TryGetValue(typeKey, out unboundGenerics))
{
// Is there an existing registration for this type?
if (unboundGenerics.Any(x => x.Type == unboundGeneric.Type))
throw new StyletIoCRegistrationException(String.Format("Multiple registrations for type {0} found", typeKey.Type.Description()));
unboundGenerics.Add(unboundGeneric);
unboundGenerics = new List<UnboundGeneric>();
this.unboundGenerics.Add(typeKey, unboundGenerics);
}
// Is there an existing registration for this type?
if (unboundGenerics.Any(x => x.Type == unboundGeneric.Type))
throw new StyletIoCRegistrationException(String.Format("Multiple registrations for type {0} found", typeKey.Type.Description()));
unboundGenerics.Add(unboundGeneric);
}
/// <summary>
/// </summary>
/// <remarks>Not thread-safe, as it's only ever called from the builder</remarks>
/// <param name="serviceType"></param>
/// <returns></returns>
internal Type GetFactoryForType(Type serviceType)
{
// Not thread-safe, as it's only called from the builder
if (!serviceType.IsInterface)
throw new StyletIoCCreateFactoryException(String.Format("Unable to create a factory implementing type {0}, as it isn't an interface", serviceType.Description()));