Unity and Random "Index was outside the bounds of the array" exception

Question

We are running web site with around 15.000 realtime user (google analytics) (Around 1000 request/sec (perf counters)).

We have two web server behind load balancer.

Sometimes every day sometimes 1 time in a week one of our web servers stop execute requests and start to response with error and every request is logging following exception: "System.IndexOutOfRangeException - Index was outside the bounds of the array."

Our environment : IIS 8.5, .Net 4.5.0, Mvc 5.1.0, Unity 3.5 (same state with 3.0), WebActivatorEx 2.0 In IIS, Worker Process 1 and other settings are with defaults.

We could not catch any specific scenario made this error. After App pool recycle everything start with no problem. And before every request respond with error, there is not any error related with it.

There is one question asked in the past with related old Unity version: https://unity.codeplex.com/discussions/328841 http://unity.codeplex.com/workitem/11791 Could not see anything I can do about it.

Here exception details:

System.IndexOutOfRangeException
Index was outside the bounds of the array.

System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at System.Collections.Generic.List`1.Enumerator.MoveNext()
   at System.Linq.Enumerable.WhereListIterator`1.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Microsoft.Practices.Unity.NamedTypesRegistry.RegisterType(Type t, String name)
   at Microsoft.Practices.Unity.UnityDefaultBehaviorExtension.OnRegisterInstance(Object sender, RegisterInstanceEventArgs e)
   at System.EventHandler`1.Invoke(Object sender, TEventArgs e)
   at Microsoft.Practices.Unity.UnityContainer.RegisterInstance(Type t, String name, Object instance, LifetimeManager lifetime)
   at Microsoft.Practices.Unity.UnityContainerExtensions.RegisterInstance[TInterface](IUnityContainer container, TInterface instance, LifetimeManager lifetimeManager)
   at DemoSite.News.Portal.UI.App_Start.UnityConfig.<>c__DisplayClass1.<RegisterTypes>b__0()
   at DemoSite.News.Portal.Core.Controller.BaseController.Initialize(RequestContext requestContext)
   at System.Web.Mvc.Controller.BeginExecute(RequestContext requestContext, AsyncCallback callback, Object state)
   at System.Web.Mvc.MvcHandler.<BeginProcessRequest>b__4(AsyncCallback asyncCallback, Object asyncState, ProcessRequestState innerState)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncVoid`1.CallBeginDelegate(AsyncCallback callback, Object callbackState)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncResultBase`1.Begin(AsyncCallback callback, Object state, Int32 timeout)
   at System.Web.Mvc.Async.AsyncResultWrapper.Begin[TState](AsyncCallback callback, Object callbackState, BeginInvokeDelegate`1 beginDelegate, EndInvokeVoidDelegate`1 endDelegate, TState invokeState, Object tag, Int32 timeout, SynchronizationContext callbackSyncContext)
   at System.Web.Mvc.MvcHandler.BeginProcessRequest(HttpContextBase httpContext, AsyncCallback callback, Object state)
   at System.Web.HttpApplication.CallHandlerExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute()
   at System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously)

My configuration is as follows:

public static void RegisterTypes(IUnityContainer container) 
{ 
    var section = (UnityConfigurationSection)ConfigurationManager.GetSection("unity");
    container.LoadConfiguration(section); 
    ServiceLocator.SetLocatorProvider(() => new UnityServiceLocator(container));
}

Initialize method as follow:

protected override void Initialize(System.Web.Routing.RequestContext requestContext)
{
    if (requestContext.RouteData.Values["ViewActionId"] != null)
    {
        int viewActionId;
        if (!int.TryParse(requestContext.RouteData.Values["ViewActionId"].ToString(), out viewActionId))
            return;

        var cacheProvider = ServiceLocator.Current.GetInstance<ICacheProvider>();
        List<ViewActionClass> viewActionClasses = null;
        string cacheKey = CacheKeyCompute.ComputeCacheKey("ViewActionClass", CacheKeyTypes.DataCache,
            new KeyValuePair<string, string>("viewActionId", viewActionId.ToString()));

        _configuration = ServiceLocator.Current.GetInstance<IConfiguration>();

        viewActionClasses = 
            cacheProvider.AddOrGetExistingWithLock<List<ViewActionClass>>(cacheKey, () =>
        {
            var viewActionClassBusiness = 
            ServiceLocator.Current.GetInstance<IViewActionClassBusiness>();

            return viewActionClassBusiness.ViewActionClassGetByViewActionId(viewActionId);
        });

        ViewBag.ActionClass = viewActionClasses;
        ViewBag.Configuration = _configuration;
    }
    base.Initialize(requestContext);
}

Registration xml for ICacheProvider, IConfiguration and IViewActionClassBusiness

<type type="DemoSite.Runtime.Caching.ICacheProvider, DemoSite.Core"
        mapTo="DemoSite.Runtime.Caching.ObjectCacheProvider, DemoSite.Core">
    <lifetime type="containerControlledLifetimeManager" />
  </type>
<type type="DemoSite.Core.Configuration.IConfiguration, DemoSite.Core"
        mapTo="DemoSite.Core.Configuration.ConfigFileConfiguration, DemoSite.Core">
    <lifetime type="containerControlledLifetimeManager" />
  </type>
<type type="DemoSite.News.Business.IViewActionClassBusiness, DemoSite.News.Business"
        mapTo="DemoSite.News.Business.Default.ViewActionClassBusiness, DemoSite.News.Business.Default">
    <lifetime type="perRequestLifetimeManager" />
  </type>

Maybe it is related with high traffic. Is there anyone encounter a problem like that and any solution ?

Thanks in advance

Answer 1

As far as I can see from the stack trace you are registering instances in the container during the web request. The RegisterType and RegisterInstance methods are not thread-safe in Unity (and this probably holds for most DI libraries in .NET). This explains why this is happening to at random points and under high load.

It is good practice to register your container only at start-up and don't change it later on. With the Dependency Inversion Principle and Dependency Injection pattern in particular you try to centralize the knowledge of how object graphs are wired, but you are decentralizing it again by doing new registrations later on. And even if registration was thread-safe with Unity, it's still very likely that you introduce race conditions by changing registrations during runtime.

UPDATE

Your code has the following code that causes the problems:

ServiceLocator.SetLocatorProvider(() => new UnityServiceLocator(container));

This seems very innocent, but in fact it causes both a concurrency bug and a memory leak.

Because the new statement is inside the lambda, it will cause a new UnityServiceLocator to be created every time you call ServiceLocator.Current. That wouldn't be bad by itself, but the UnityServiceLocator's constructor makes a call to container.RegisterInstance to register itself in the container. But as I already said: calling RegisterInstance` is not thread-safe.

But even if it was thread-safe, it still causes a memory leak in your application, since a call to RegisterInstance will not replace an existing registration, but appends it to a list of registrations. This means that the list of UnityServiceLocator instances in the container will keep growing and will eventually cause the system to crash with an OutOfMemoryException. You are actually lucky that you hit this concurrency bug first, because the OOM bug would be much harder to trace back.

The fix is actually very simple: move the construction of the UnityServiceLocator out of the lambda and return that single instance every time:

var locator = new UnityServiceLocator(container);
ServiceLocator.SetLocatorProvider(() => locator);

The behavior of the UnityServiceLocator is a design flaw in my opinion, because since RegisterInstance is not thread-safe and the UnityServiceLocator has no idea how many times it is created, it should never call RegisterInstance from within its constructor -or at least- not without doing a check whether it is safe to register that instance.

Problem however is that removing that call to RegisterInstance is a breaking change, but still probably the best solution for the Unity team. Most users will probably not notice the missing IServiceLocator registration anyway and if they do, Unity will communicate a clear exception message in that case. Another option would be to let the UnityServiceLocator check whether any instances have already been resolved from the container, and in that case throw an InvalidOperationException from within the UnityServiceLocator's constructor.