3

We recently developed a new ASP.NET MVC 4 web application (C#/Visual Studio). After local testing and debugging we deployed it to production, and then started getting more and more health monitoring mails. These had different Exception messages:

  • Stack Empty.
  • Collection was modified; enumeration operation may not execute.
  • Item has already been added. Key in dictionary: 'ALL_HTTP' Key being added: 'ALL_HTTP' (other keys also mentioned).
  • Value does not fall within the expected range.

E.g. a whole series of error types we could not simply resolve or reproduce. The 'Stack Empty' is the one occurring most, several 100 times per day (e.g. for 1-10% of users) so we focus on this one, as the other errors seem related. Here is a partial stack trace:

Exception information:
Exception type: System.InvalidOperationException
Exception message: Stack empty.
...
Stack trace:    at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
at System.Collections.Generic.Stack`1.Pop()
at System.Web.WebPages.TemplateStack.Pop(HttpContextBase httpContext)

As shown stack trace are mostly located completely in the MVC framwork (System.Web). The only place in our own code that regularly occured in some stack traces were in the views (.cshtml files) of the requested URL and then in a @Html.RenderAction() call. By now we have refactored a lot of these to RenderPartial() calls. This lead to no more views in the stack trace, though some RenderPartial now also gave some

Searching for this error indicated concurrency/parallel execution is the cause. This matches the fact that we initially could not reproduce the error locally, but it did happen on production. We have done no load testing, but by now have been able to reproduce the error on a local developer system by starting a lot of applications/requests simultaneously. However in our code NOTHING is done with explicit parallel instructions.

This seems to be related with MVC view's NOT being thread safe. However it is hard to imagine nobody else would have encountered this. We have a few thousand visitors a day, roughly 30+ active users at any moment. Sadly this number is now falling due to decreasing Google ranking (related to this problem).

Does anyone knows a solution/approach to this problem?

Bart
  • 5,065
  • 1
  • 35
  • 43
  • 1
    Are you passing as model any object that could be potentially accessed by multiple threads? Like a shared object? – vtortola Dec 10 '12 at 11:00
  • vtorola Not really! Wel all models do extend from BaseModel that has a static db context object, However this is stored in the HTTPContext (using a key). This is done so it is used on a 'per request' basis (it is lazy loaded on the first access). Stack traces do NOT point to this code, they are only in the view logic. Furthermore I have previously already added a 'lock' statement on the code that iniially sets the DB Context, just in case. This did not appear to change anything. We are using Entity Framework. – Bart Dec 10 '12 at 11:26
  • Woa that sounds creepy :D You should use your DbContext as part of your controller, generate the desired POCO objects from the DB, and then create your view models. A view should be thread safe, I have never seen an error like that, so I would suggest to start looking at that creepy shared DbContext deeply. Maybe the stack trace does not point to that BaseModel object, but definitely a part of it is being shared across multiple threads. – vtortola Dec 10 '12 at 11:52
  • Well A database on a 'per request basis' is often used. Im following the 'Unit of work' pattern. See http://blog.stevensanderson.com/2007/11/29/linq-to-sql-the-multi-tier-story/ Changing this would require me to change all static DB access functions to dynamic ones. This construct worked perfectly for years on the ASP.NET webforms version of this application, even till this day, as we have thousands of users still on this site. (And if we don't fix this problem they won't be switched to the new website :). – Bart Dec 10 '12 at 13:16
  • 1
    Oh yes that is ok. What I mean is that models should be pretty much simple constructions that represents the state of a view, no complex hierarchies able of creating their own DB connections. It was just a suggestion to make thins clearer. To have a DbContext per request I use a IoC/DI framework. – vtortola Dec 10 '12 at 13:27

2 Answers2

3

I am developing a ASP.NET MVC 4 application and I also came across the errors that you mention. Although they are different they seem to have the same source. After spending several hours trying to find the reason (and a lot of code changes) I have started my analysis from scratch.

Since I am using a Custom Route and there is a handler for that route that checks several things and also accesses the database I started by commenting database access. Opening several browser tabs very quickly (with IISExpress > Show All Application window or by Ctrl+Click in a link) I was happy to see that all the pages were shown properly, instead of several random error messages. Tried that a few times to be sure and concluded that something was wrong while accessing the DB.

public class MyNewRouteHandler : IRouteHandler {

    IHttpHandler MvcHandler;

    public IHttpHandler GetHttpHandler(RequestContext requestContext) {
        MvcHandler = new MvcHandler(requestContext);

        // some checkings and
        // some database access code 
        // that was commented

        return MvcHandler;
    }
}

A colleague suggested that I added a small Thread sleep inside this method: GetHttpHandler. That line made the errors appear again, suggesting that the problem was not related to DB... When I did that I saw that MvcHandler object was being defined as a class property and that could be a source of what appeared to be a concurrency issue (only when multiple almost consecutive accesses were executed, the problem was shown). Moved the MvcHandler object to a local object inside the method.

public class MyNewRouteHandler : IRouteHandler {

    public IHttpHandler GetHttpHandler(RequestContext requestContext) {
        IHttpHandler MvcHandler = new MvcHandler(requestContext);

        // some checkings and
        // some database access code 
        // that was commented

        return MvcHandler;
    }
}

And after testing, no more errors. Uncommented all my code that accessed the DB (and did other checkings) and still no more errors found. Almost 3 days have gone by and everything still working properly.

nrod
  • 398
  • 6
  • 17
  • Great nrod! Indeed I'm looking at it, but also seem to use a custom MVC HttpHandler. And these guys don't seem to be thread safe... Tough one to found, since the stack traces weren't pointing to the routing at all. – Bart Dec 12 '12 at 16:32
0

This way of doing a Custom Route Handler did solve my most of my errors but I still have a few left and with new messages. One of them pointed to a code line in my Custom Route Handler and all of them had in common the fact that a Dictionary was being handled by the MVC framework, so... do I still have a concurrency problem?

I assumed so and all my method properties were moved inside the public IHttpHandler GetHttpHandler(RequestContext requestContext) method, not only the one mentioned before. One of them was the RouteData collection... Finally and after 2 days it seems that no more errors are showing.

nrod
  • 398
  • 6
  • 17