What are the dangers of using Session.SyncRoot for

2019-07-20 22:31发布

问题:

I have a race condition with the following code if two requests come in really close together in an ASP.NET MVC app:

var workload = org.Workloads.SingleOrDefault(p => ...conditions...);
if (workload == null) {
    workload = org.CreateWorkload(id);
}

workload and org are EntityFramework objects. The call to CreateWorkload adds a row to a Workloads table in the database. (We really should enforce this with a UNIQUE constraint on the table, but I can't now that the table has some dirty data in it.) Subsequent calls to the Action method that contains this code throws an exception when SingleOrDefault encounters more than one row satisfying the conditions.

So to fix this, I want to lock these lines of code. I don't want it done per request, with static lock object because that slows the site down for every user. What I'd like to do is use Session.SyncRoot for locking. I.e.

Workload workload;
lock (Session.SyncRoot) 
{
    workload = org.Workloads.SingleOrDefault(p => ...conditions...);
    if (workload == null) {
        workload = org.CreateWorkload(id);
    }
}

I'm not an ASP.NET expert, however, and there are some warning signs showing up in the docs and ReSharper, namely, that it can throw NotImplementedExceptions or be null. However, testing shows that this works just fine.

So, ASP.NET experts, what are the risks of using Session.SyncRoot for this? As an alternative, if Session.SyncRoot is "really risky", could I assign a lock object in the Session collection on Session start up to do the same thing?

回答1:

The danger only exists if you use a custom session class that implements HttpSessionStateBase but doesn't override the SyncRoot property to do something other than throw a NotImplementedException. The HttpSessionStateWrapper class and the HttpSessionState class DO implement and override the SyncRoot method. So, as long as you're accessing the Session via the HttpSessionStateWrapper or HttpSessionState classes and not a custom class, this will work just fine.