Adil Mammadov
Adil Mammadov

Reputation: 8706

Locking entity in MVC

Short question

How can I lock my entity so that only one operation by only one user can be performed on it at a time in MVC project?

Long question

I have MVC project where I want my action methods to be [SessionState(SessionStateBehavior.ReadOnly)]. But when doing this users can execute another action methods even before one long running action method has not completed. As I have a lot calculations and action methods have to be executed in predefined order, executing another Action method before one ends creates lots of problems. To give example I have main entity called Report, I have to somehow ensure that one report undergoes only one operation by only one user at a time. So I have to lock my Report. Even if I do not use [SessionState(SessionStateBehavior.ReadOnly)] I have to lock report so that multiple users do not edit same reports at a time and for other specific reasons. Currently I am writing this information to database roughly something like:

ReportId
LockedUserId
IsInPorcess

I have to set IsInProcess to true every time before operation begins and reset it to false after operation completed. As I have lots of action methods I created ActionFilter something like below:

public class ManageReportLockAttribute
    : FilterAttribute, IActionFilter
{
    public ManageReportLockAttribute()
    {       
    }

    public void OnActionExecuting(ActionExecutingContext filterContext)
    {
        ...
        ReportLockInfo lockInfo = GetFromDatabase(reportId);
        if(lockInfo.IsInProcess)
            RedirectToInformationView();

        lockInfo.IsInProcess = true;
        SaveToDatabase(lockInfo);
        ...
    }

    public void OnActionExecuted(ActionExecutedContext filterContext)
    {       
        ...
        ReportLockInfo lockInfo = GetFromDatabase(reportId);
        lockInfo.IsInProcess = false;
        SaveToDatabase(lockInfo);
        ...
    }
}

It works, for most part, but it has some strange problems (see this question for more info). My question is that "How can I achieve same functionality (locking report) by different more acceptable way?". I feel like it is something similar to locking when using multithreading, but it is not exactly same IMO.

Sorry for long, broad and awkward question, but I want a direction to follow. Thanks in advance.

Upvotes: 1

Views: 1200

Answers (1)

Markus
Markus

Reputation: 22501

One reason why OnActionExecuted is not called though OnActionExecuting runs as expected is that there are unhandled exceptions that occur in OnActionExecuting. Especially when dealing with the database, there are various reasons that could lead to an exception, e.g.:

  • User1 starts the process and locks the entity.
  • User2 also wants to start the process before User1 has saved the change. So the check of IsInProcess does not lead to the redirection and User2 also wants to save the lock. In this case, a concurrency violation should occur because User1 has saved the entity in the meantime.

To illustrate the process over time (C is the check whether IsInProcess is set, S is SaveChanges): first a good case:

User1     CS  
User2       CS (check is done after save, no problem)

Now a bad case:

User1     CS  
User2      CS (check takes place after check for User1, but before SaveChanges becomes effective ==> concurrency violation)

As the example shows, it is critical to make sure that only one user can place the lock. There are several ways to handle this. In all cases make sure that there are as few reasons for exceptions in OnActionExecuting as possible. Handle and log the exceptions.

Please note that all synchronisation methods will have a negative impact on the performance of your application. So if you haven't already thought about whether you could avoid having to lock the report by restructuring your actions or the data model, this would be the first thing to do.

Easy approach: thread synchronisation

An easy approach is to use thread synchronisation. This approach will only work if the application runs in a single process and not in a web farm/the cloud. You need to decide whether you will be able to change the application if it will be installed in a farm at a later point in time. This sample shows an easy approach (that uses a static object for locking):

public class ManageReportLockAttribute
    : FilterAttribute, IActionFilter
{
    private static readonly object lockObj = new object();
    // ...    
    public void OnActionExecuting(ActionExecutingContext filterContext)
    {
        ...
        ReportLockInfo lockInfo = GetFromDatabase(reportId);
        if(lockInfo.IsInProcess)
          RedirectToInformationView();
        lock(lockObj)
        {
          // read anew just in case the lock was set in the meantime
          // A new context should be used.
          lockInfo = GetFromDatabase(reportId); 
          if(lockInfo.IsInProcess)
            RedirectToInformationView();
          lockInfo.IsInProcess = true;
          SaveToDatabase(lockInfo);
          ...
        }
    }

    public void OnActionExecuted(ActionExecutedContext filterContext)
    {
      ...
      lock(lockObj)
      {
        lockInfo = GetFromDatabase(reportId);
        if (lockInfo.IsInProcess) // check whether lock was released in the meantime
        {
          lockInfo.IsInProcess = false;
          SaveToDatabase(lockInfo);
        }
        ...
      }
    }
}

For details on using lock see this link. If you need more control, have a look at the overview of thread synchronization with C#. A named mutex is an alternative that provides locking in a more fine coarsed manner.

If you want to lock on reportId instead of a static object, you need to use a lock object that is the same for the same reportId. A dictionary can store the lock objects:

private static readonly IDictionary<int, object> lockObjectsByReportId = new Dictionary<int, object>();

private static object GetLockObjectByReportId(int reportId)
{
  int lockObjByReportId;
  if (lockObjectsByReportId.TryGetValue(reportId, out lockObjByReportId))
    return lockObjByReportId;
  lock(lockObj)  // use global lock for a short operation
  {
    if (lockObjectsByReportId.TryGetValue(reportId, out lockObjByReportId))
      return lockObjByReportId;
    lockObjByReportId = new object();
    lockObjectsByReportId.Add(reportId, lockObjByReportId);
    return lockObjByReportId;
  }
}

Instead of using lockObj in OnActionExecuting and OnActionExecuted, you'd use the function:

// ...
lock(GetLockObjectByReportId(reportId))
{
  // ...
}

Database approach: Transactions and isolation levels

Another way to handle this is to use database transactions and isolation levels. This approach will also work in a multi-server environment. In this case, you'd not use the entity framework for database access but move the code to a stored procedure that is run on the database server. By running the stored procedure in a transaction and picking the right isolation level, you can avoid that a user can read the data while another one is changing them.

This link shows an overview of isolation levels for SQL Server.

Upvotes: 1

Related Questions