unitycontainer / abstractions Goto Github PK

Allow arbitrary Delegate to serve as Injection Factory. Allow container to resolve required arguments while calling the factory method.

It should be possible to provide Type and Name as two separate arguments without allocating new NamedTypeBuildKey(). The interface should change from this:

object NewBuildUp(INamedType newBuildKey, Action<IBuilderContext> childCustomizationBlock = null)

To this:

object NewBuildUp(Type type, string name, Action<IBuilderContext> childCustomizationBlock = null)

For backwards compatibility old API call should be implemented as extension method.

@abramovandrey Wrote:
Please make Unity assemblies [SecurityTransparent] (at least lifetime managers) as it was in 4.0.1
Moved from unitycontainer/unity#130

The registration looks something like this:

container.RegisterType<IFoo, Foo>(new InjectionFactory(cont => FooFactory.Create()));

When resolving IFoo the InjectionFactory isn't being called, which results in an InvalidOperationException: "The type Foo does not have an accessible constructor."

Looking at the registration in the debugger, I see the "IFoo" registration has two IBuilderPolicy values associated:

1. IBuildPlanPolicy => ResolveBuildUpPolicy
2. IBuildKeyMappingPolicy => BuildKeyMappingPolicy

I was expecting to find a registration like "IBuildPlanPolicy => InjectionFactory"

Currently PreBuildUp and PostBuildUp have no direct communication between each other. Every policy resolved in PreBuildUp must be resolved in PostBuildUp as well.
For example lifetime policy:

public override void PreBuildUp(IBuilderContext context)
{
    var lifetimePolicy = GetLifetimePolicy(context, out _);
   ... 

First thing it does, it retrieves lifetime policy for the registration. When it exits that policy is released and PostBuildUp must retrieve it again:

public override void PostBuildUp(IBuilderContext context)
{
    var lifetimePolicy = GetLifetimePolicy(context, out _);
    lifetimePolicy?.SetValue(context.Existing, context.Lifetime);
}

This results in numerous calls to hierarchical dictionary and a lot wasted cycles. Instead it should be possible to pass the policy from PreBuildUp to PostBuildUp via return result of PreBuildUp and extra parameter on PostBuildUp. The interface should change like this:

public interface IBuilderStrategy
{
    object PreBuildUp(IBuilderContext context);
    void PostBuildUp(IBuilderContext context, object pre);
}

Removing extensions does not make much sense. Creating new container is much cleaner.

This comes as part of ClearExistingBuildPlan cleanup effort in unitycontainer/container#37.

IConstructorSelectorPolicy has method SelectConstructor which returns solid type SelectedConstructor. To enable further development and optimization it should return interface to allow alternative implementations.

SelectedConstructor SelectConstructor(IBuilderContext context, IPolicyList list);

Should be:

ISelectedConstructor SelectConstructor(IBuilderContext context, IPolicyList list);

At present members of ILifetimePolicy passed reference to Lifetime manager of resolving container. To allow more optimization it should receive IBuilderContext instead:

public interface ILifetimePolicy : IBuilderPolicy
{
    /// <summary>
    /// Retrieve a value from the backing store associated with this Lifetime policy.
    /// </summary>
    /// <param name="container">The container this lifetime belongs to</param>
    /// <returns>the object desired, or null if no such object is currently stored.</returns>
    object GetValue(IBuilderContext context = null);

    /// <summary>
    /// Stores the given value into backing store for retrieval later.
    /// </summary>
    /// <param name="newValue">The object to store.</param>
    /// <param name="container">The container this lifetime belongs to</param>
    void SetValue(object newValue, IBuilderContext context = null);

    /// <summary>
    /// Remove the value this lifetime policy is managing from backing store.
    /// <param name="container">The container this lifetime belongs to</param>
    /// </summary>
    void RemoveValue(IBuilderContext context = null);
}

Add 'official' inward looking interface to the container.

This interface should be a standard (same as IUnityContainer is for outside world) internal window to the container's engine.

/// <summary>
/// Container interface exposing engine to internal container parts 
/// </summary>
public interface IContainerContext
{
    /// <summary>
    /// The container that this context is associated with.
    /// </summary>
    /// <value>The <see cref="IUnityContainer"/> object.</value>
    IUnityContainer Container { get; }

    /// <summary>
    /// The <see cref="ILifetimeContainer"/> that this container uses.
    /// </summary>
    /// <value>The <see cref="ILifetimeContainer"/> is used to manage <see cref="IDisposable"/> objects that the container is managing.</value>
    ILifetimeContainer Lifetime { get; }

    /// <summary>
    /// The policies this container uses.
    /// </summary>
    /// <remarks>The <see cref="IPolicyList"/> the that container uses to build objects.</remarks>
    IPolicyList Policies { get; }

}

This interface is different from ExtensionContext in usage pattern. ExtensionContext exposes Strategies, Defaults and etc. and mainly used to configure and extend the container.
This inerface exposes only parts required for operating already configured container.

This lifetime manager should be globally unique singleton and should be the same no matter where in hierarchy it is created or retrieved.

This policy is going to be used for more than dependencies and should be called appropriately.
For legacy support IDependencyResolverPolicy derived from IResolverPolicy will be created withing Unity 5 container.

Accessing IPolicyList requires new allocations of Key object and contributes to performance penalties. This could be avoided if Type and Name were part of API. So instead of:

IBuilderPolicy Get(Type policyInterface, object buildKey, out IPolicyList containingPolicyList);
void Set(Type policyInterface, IBuilderPolicy policy, object buildKey = null);
void Clear(Type policyInterface, object buildKey);

it should be:

IBuilderPolicy Get(Type type, string name, Type policyInterface, out IPolicyList list);
void Set(Type type, string name, Type policyInterface, IBuilderPolicy policy);
void Clear(Type type, string name, Type policyInterface);

IPropertySelectorPolicy has method SelectProperties which returns solid type SelectedProperty. To enable further development and optimization it should return enumerator on interface to allow alternative implementations.

IEnumerable<SelectedProperty> SelectProperties(IBuilderContext context, IPolicyList list);

Should be:

IEnumerable<ISelectedProperty> SelectProperties(IBuilderContext context, IPolicyList list);

Hi,

Please can you release a consistent set of NuGet packages where they are all using Unity.Abstractions 2.1.1.0?

Unity.Container depends on Unity.Abstractions 2.1.1.0 but NuGet is only on Unity.Abstractions 2.1.0.0

Having a little version hell and fiddling the app.config/web.config dependentAssembly settings is breaking my head.

Thanks,
Paul

These null checks can contribute up to 5% of resolution time if done in release mode.
All extension methods should be only checked in Debug build

At times BuildKeyMappingStrategy strategy needs to be told it need resolve instead of build path.

At present StagedStrategyChain accepts only strategies derived from IBuilderStrategy. To allow this mechanism to host multiple strategy types (Constructor selectors, Property selectors, and etc.) it should be mage generic so it would accept any type. So

IStagedStrategyChain<TStageEnum>

becames

IStagedStrategyChain<TStrategy,TStageEnum>

IBuilderStrategy is no longer required. Just use IEnemerable<...> to iterate over strategies.

There are absolutely no reason why these extra types should be allocated. All arguments could be referenced withing existing types without any overhead.

I believe you have introduced a breaking change in which the UnityCOntainerExtensions.ResolveAll does not call the correct method. See: https://github.com/unitycontainer/abstractions/blob/master/src/Utility/UnityContainerExtensions.cs#L519

The line should read
var result = (container ?? throw new ArgumentNullException(nameof(container))).ResolveAll((type ?? throw new ArgumentNullException(nameof(type

The key method missing being ResolveAll.

This was discovered when Prism upgraded to v5 and our unit tests started failing.

Current implementation of IUnityContainer registration function is not uniform and confusing. As of now it is declared as this:

/// <summary>
        /// Register a type mapping with the container, where the created instances will use
        /// the given <see cref="LifetimeManager"/>.
        /// </summary>
        /// <param name="typeFrom"><see cref="Type"/> that will be requested.</param>
        /// <param name="typeTo"><see cref="Type"/> that will actually be returned.</param>
        /// <param name="name">Name to use for registration, null if a default registration.</param>
        /// <param name="lifetimeManager">The <see cref="LifetimeManager"/> that controls the lifetime
        /// of the returned instance.</param>
        /// <param name="injectionMembers">Injection configuration objects. Can be null.</param>
        /// <returns>The <see cref="IUnityContainer"/> object that this method was called on (this in C#, Me in Visual Basic).</returns>
        IUnityContainer RegisterType(Type typeFrom,  
                                     Type typeTo, 
                                     string name, 
                                     LifetimeManager lifetimeManager, 
                                     params InjectionMember[] injectionMembers);

If I want to register mapping from IFoo to Foo I would use this form:

container.RegisterType(typeof(IFoo), typeof(Foo), ...);

In this registration we have Registered Type typeFrom (IFoo) mapped to typeTo (Foo).

But if we do not have mapping and just want to register type we would use following form:

container.RegisterType(null, typeof(Foo), ...);

In this case Registered Type is typeTo (Foo).

So, as you can see depending on mapping registered type is either typeTo or typeFrom.

To eliminate this ambiguity new interface is proposed. Method RegisterType(...) will be replaced with:

/// <summary>
/// Register a type mapping with the container, where the created instances will use
/// the given <see cref="LifetimeManager"/>.
/// </summary>
/// <param name="registeredType"><see cref="Type"/> that will be requested.</param>
/// <param name="name">Name to use for registration, null if a default registration.</param>
/// <param name="mappedTo"><see cref="Type"/> that will actually be returned.</param>
/// <param name="lifetimeManager">The <see cref="LifetimeManager"/> that controls the lifetime
/// of the returned instance.</param>
/// <param name="injectionMembers">Injection configuration objects. Can be null.</param>
/// <returns>The <see cref="IUnityContainer"/> object that this method was called on (this in C#, Me in Visual Basic).</returns>
IUnityContainer RegisterType(Type registeredType, 
                             string name, 
                             Type mappedTo, 
                             LifetimeManager lifetimeManager, 
                             params InjectionMember[] injectionMembers);

With this change new syntax for registering both cases would be:

container.RegisterType(typeof(IFoo), name, typeof(Foo), ...);
container.RegisterType(typeof(Foo), name, null, ...);

In this registration one can clearly see what is registered and how it is mapped.

Support for additional selector strategies should be added.

ConstructorSelectStage

• Setup
• Override
• Injection
• Attribute
• Default

PropertySelectStage

• Setup
• Override
• Injection
• Attribute
• Default

MethodSelectStage

• Setup
• Injection
• Override
• Attribute
• Default

It is faster to resolve unregistered POCO type

Every policy in Unity operates on instances of solid type NamedTypeBuildKey. This requires allocating new instances every time type or name is used.

Using solid type does not allow optimizations where caller might already have all required information and could be simply cast into required type/name combo. To resolve this issue NamedTypeBuildKey should be changed to interface INamedType. Instead of

NamedTypeBuildKey SomePolicy(NamedTypeBuildKey buildKey, IBuilderContext context);

it should became

INamedType SomePolicy(INamedType buildKey, IBuilderContext context);

IBuilderContext should also use INamedType instead of NamedTypeBuildKey

public interface IBuilderContext
{
    ...
    INamedType OriginalBuildKey { get; }
    INamedType BuildKey { get; set; }
    ...

IMethodSelectorPolicy has method SelectMethods which returns enumerator on solid type SelectedMethod. Instead it should return enumerator of ISelectedMethod interface.

IEnumerable<SelectedMethod> SelectMethods(IBuilderContext context, IPolicyList list);

Should be:

IEnumerable<ISelectedMethod> SelectMethods(IBuilderContext context, IPolicyList list);

Normally Unity container does not keep track of created transient objects. It creates them and forgets them. In certain situations it is useful to make container remember all transient objects it have created.

So, ContainerControlledTransientManager is required. This lifetime manager is the same as TransientLifetimeManager except if the object implements IDisposable it will keep strong reference to object and dispose it when container is disposed.
If created object is not disposable, container does not maintain any object references so when that object is released GC will collect it immediately.

Method IsRegistered with signature:

bool IsRegistered(Type type, string name)

should be added to IUnityContainer interface.

@pbaravik wrote:

To my mind the code of the method SynchronizedLifetimeManager.GetValue() contains potentially unsafe code:

public override object GetValue()
{    
  Monitor.Enter(this.lockObj);    
  object obj2 = this.SynchronizedGetValue();    
  if (obj2 != null)    
  {
    Monitor.Exit(this.lockObj);    
  }    
  return obj2;
}

The scenario might be the following:

• the thread captures the lock object
• SynchronizedGetValue() returns null
  • I did not find that Recovery would be called if null resolved and thus sync block remains captured
• the thread leaves Unity code
• the thread is terminated for any reason
• the deadlock situation will take place.

Sample output of WinDbg SOS extensions deadlock analysis (real case):

0:045> !dlk
Examining SyncBlocks...
Scanning for ReaderWriterLock(Slim) instances...
Scanning for holders of ReaderWriterLock locks...
Scanning for holders of ReaderWriterLockSlim locks...
Examining CriticalSections...
Scanning for threads waiting on SyncBlocks...
Scanning for threads waiting on ReaderWriterLock locks...
Scanning for threads waiting on ReaderWriterLocksSlim locks...
Scanning for threads waiting on CriticalSections...
DEADLOCK DETECTED
CLR Thread 0x14 is waiting for orphaned SyncBlock 0000000012a3f508 OBJ:000000037f47c750[System.Object]
The lock was orphaned by CLR thread 0x2ef, which has since terminated.
CLR Thread 0x14 is waiting at Microsoft.Practices.Unity.SynchronizedLifetimeManager.GetValue()(+0xb IL,+0x12 Native)

1 deadlock detected.

It should be possible to traverse IBuilderContext chain to verify cyclical dependencies and other abnormal conditions.