One long running process per user in an ASP.NET Core Application

Question

I have an async function which will go and do some long running and CPU intensive task. As it is async it doesn't hold up the UI.

Under normal conditions I have no problem with the way it runs. The process is triggered on a button click, but it could be triggered on an API call.

I found that if I hit the button many many times in quick succession, the whole site starts to respond slowly.This is an abusive type of use but nevertheless it has a detrimental impact on performance.

I would like to implement a list in memory of calls to functions by user so that each user would be able to have a single instance the long running function running, but it would not accept any further requests from that user.

Who knows of some clever way of implementing this or has done something like that before. I'm thinking of something like a threadsafe collection you can add tasks to, but if you try to .Add a task that is already running for a certain user / claimsprincipal, then it will fail to add it.

Answer 1

There are some issues with your solution:

1. If the task is CPU intensive it can still hold up the UI even when it is async. Once all CPUs are busy, you will see a performance impact.
2. Long running taks are not a good idea on a webserver. Webservers are designed to handle a large number of short requests. Long requests are not handled efficiently and they might also be recycled before they have completed.
3. Your implementation makes denial of service attacks easy. You basically have created one with clicking your button multiple times.

You should offload these kinds of tasks to an application server that is better suited to handle this kind of work. That keeps your webserver free to do what it does best: handle web requests.

Use the form of communication between web and application server that suits your needs best: WCF, a REST API or a message queueing framework such as MSMQ or RabbitMQ.

Answer 2

Without diving into the question of whether doing this is the right thing for your specific business situation and architecture, I see your question as basically boiling down to this: How can I ensure that a user can only invoke a particular process one at a time.

You could use a threadsafe collection to hold a flag for each user that indicates whether they are currently running such a process. Use a try{}finally{} block in your async code to manage the value. Set it to true inside the try block and set it to false in the finally. The rest of your long running work comes after you set it to true and before you the finally is executed. sketched out below:

try{
    //if thread safe value for this user is already true then exit.

    //set thread safe collection value for this user to true

    //Do work

 }finally{

     //set thread safe collection value for this user to false;
 }

If it were me, I'd put an object in the collection where the object has one value which was the bool. That way I could use the lock keyword to hold a lock on the object for that user when I was checking it's value or setting it's value so that I could ensure that no other process changed the value between when I checked it and when I set it.

Hope that helps.

Answer 3

In the end I implemented a Service as a Singleton that I registered with DI.

public class SingleInstanceRegisterService: IDisposable
{
    private ConcurrentDictionary<SingleInstance,SingleInstance> dict = new ConcurrentDictionary<SingleInstance, SingleInstance>();
    AutoResetEvent arv;
    Timer timer;

    public SingleInstanceRegisterService()
    {
        arv = new AutoResetEvent(false);
        timer = new Timer(this.Cleanup, arv, 0, 60000);
    }

    public bool TryAdd(SingleInstance i)
    {
        return dict.TryAdd(i, i);
    }

    public bool ContainsKey(SingleInstance i)
    {
        return dict.ContainsKey(i);            
    }

    public bool TryRemove(SingleInstance i)
    {
        SingleInstance x;
        return dict.TryRemove(i, out x);
    }

    public void Cleanup(Object stateInfo)
    {
        AutoResetEvent autoEvent = (AutoResetEvent)stateInfo;

        if (dict != null)
        {
            foreach(SingleInstance i in dict.Keys)
            {
                if (i.Expiry < DateTimeOffset.Now)
                {
                    TryRemove(i);
                }
            }

        }
        autoEvent.Set();
    }

    public void Dispose()
    {
        timer?.Dispose();
        timer = null;
    }
}

The SingleInstance is then defined as:

 public class SingleInstance: IEquatable<SingleInstance>
{
    public string UserSubject;
    public string FullMethodName;
    public DateTimeOffset Expiry;

    public SingleInstance(User u, MethodInfo m, DateTimeOffset exp)
    {
        UserSubject = u.UserSubject.ToString();
        FullMethodName = m.DeclaringType.FullName + "." + m.Name;
        Expiry = exp;
    }

    public bool Equals(SingleInstance other)
    {
        return (other != null &&
            other.FullMethodName == this.FullMethodName &&
            other.UserSubject == this.UserSubject);
    }

    public override bool Equals(object other)
    {
        return this.Equals(other as SingleInstance);
    }

    public override int GetHashCode()
    {
        return (UserSubject.GetHashCode() + FullMethodName.GetHashCode()); 
    }
}

To use it we do the following. _single is my dependency injected SingleInstanceRegisterService.

var thisInstance = new SingleInstance(user, method,   DateTimeOffset.UtcNow.AddMinutes(1));
if (_single.TryAdd(thisInstance))
{
    // tell the database to kick off some long running process
    DoLongRunningDatabaseProcess();
    // remove lock on running process
    _single.TryRemove(thisInstance);
}

If the TryAdd fails to add it then it must already be running. Note that there is a timer that will automatically clear out process locks that have expired.