VansFannel
VansFannel

Reputation: 45911

Lock Web API controller method

I'm developing an ASP.NET Web Api application with C# and .Net Framework 4.7.

I have a method in a controller that I want to execute only by one thread at a time. In other words, if someone calls this method, another call must wait until the method has finished.

I have found this SO answer that could do the job. But here it uses a queue and I don't know how to do it to consume that queue. In that answer explains that I can create a windows service to consume the queue but I don't want to add another application to my solution.

I thought to add a lock inside the Web Api method like this:

[HttpPut]
[Route("api/Public/SendCommissioning/{serial}/{withChildren}")]
public HttpResponseMessage SendCommissioning(string serial, bool withChildren)
{
    lock
    {
        string errorMsg = "Cannot set commissioning.";

        HttpResponseMessage response = null;
        bool serverFound = true;

        try
        {
            [ ... ]
        }
        catch (Exception ex)
        {
            _log.Error(ex.Message);

            response = Request.CreateResponse(HttpStatusCode.InternalServerError);
            response.ReasonPhrase = errorMsg;
        }

        return response;
    }
}

But I don't think this is a good solution because it could block a lot of pending calls if there is a problem running the method and I will lost all the pending calls or maybe I'm wrong and the calls (threads) will wait until the others end. In other words, I think if I use the this I could reach a deadlock.

I'm trying this because I need to execute the calls in the same order I receive it. Look at this action log:

2017-06-20 09:17:43,306 DEBUG [12] WebsiteAction - ENTERING PublicController::SendCommissioning , serial : 38441110778119919475, withChildren : False
2017-06-20 09:17:43,494 DEBUG [13] WebsiteAction - ENTERING PublicController::SendCommissioning , serial : 38561140779115949572, withChildren : False
2017-06-20 09:17:43,683 DEBUG [5] WebsiteAction - ENTERING PublicController::SendCommissioning , serial : 38551180775118959070, withChildren : False
2017-06-20 09:17:43,700 DEBUG [12] WebsiteAction - EXITING PublicController::SendCommissioning 
2017-06-20 09:17:43,722 DEBUG [5] WebsiteAction - EXITING PublicController::SendCommissioning 
2017-06-20 09:17:43,741 DEBUG [13] WebsiteAction - EXITING PublicController::SendCommissioning 

I receive three calls before any of them end: threads [12], [13] and [5]. But the last one ends before the second one: [12], [5] and [13].

I need a mechanism to don't allow this.

What can I do to ensure that the calls will be process in the same order that I made them?

Upvotes: 21

Views: 21858

Answers (2)

Jim Aho
Jim Aho

Reputation: 11867

I guess you can lock on different levels, with your approach being one.

I've come across a system (or a web app) that utilizes redis as an external service. In redis we save a key for the request, in your case I guess it could be the name of the method. With this approach we first have an action filter that checks if a lock exists (talking to redis) and then blocks the request.

The good thing with redis is that it's very fast, and let's your specify a timeout at which the key will dissapear. This prevents from being stuck with the lock forever.

Upvotes: 0

Blue
Blue

Reputation: 22911

Your lock solution should work fine. If the request fails, then the lock will be released, and other pending requests can then enter the lock. A deadlock wont occur.

The only issue with this solution, is web requests will go on hanging for possibly long periods of time (Which may result in time outs from the client end).

public class MyApi : ApiController
{
    public static readonly object LockObject = new object();

    [HttpPut]
    [Route("api/Public/SendCommissioning/{serial}/{withChildren}")]
    public HttpResponseMessage SendCommissioning(string serial, bool withChildren)
    {
        lock ( LockObject )
        {
            //Do stuff
        }
    }
}

To solve the issue with hanging requests, you should utilize a queue, and poll the back end (Or if you're fancy, try SignalR) until your job is complete. For example:

//This is a sample with Request/Result classes (Simply implement as you see fit)
public static class MyBackgroundWorker
{
    private static ConcurrentQueue<KeyValuePair<Guid, Request>> _queue = new ConcurrentQueue<KeyValuePair<Guid, Result>>()
    public static ConcurrentDictionary<Guid, Result> Results = new ConcurrentDictionary<Guid, Result>();

    static MyBackgroundWorker()
    {
         var thread = new Thread(ProcessQueue);
         thread.Start();
    }

    private static void ProcessQueue()
    {
         KeyValuePair<Guid, Request> req;
         while(_queue.TryDequeue(out req))
         {
             //Do processing here (Make sure to do it in a try/catch block)
             Results.TryAdd(req.Key, result);
         }
    }

    public static Guid AddItem(Request req)
    {
        var guid = new Guid();
        _queue.Enqueue(new KeyValuePair(guid, req));
        return guid;
    }
}


public class MyApi : ApiController
{
    [HttpPut]
    [Route("api/Public/SendCommissioning/{serial}/{withChildren}")]
    public HttpResponseMessage SendCommissioning(string serial, bool withChildren)
    {
        var guid = MyBackgroundWorker.AddItem(new Request(serial, withChildren));
        return guid;
    }

    [HttpGet]
    [Route("api/Public/GetCommissioning/{guid}")]
    public HttpResponseMessage GetCommissioning(string guid)
    {
        if ( MyBackgroundWorker.Results.TryRemove(new Guid(guid), out Result res) )
        {
            return res;
        }
        else
        {
            //Return result not done
        }
    }
}

Upvotes: 18

Related Questions