Task Wrappers for starting operation only if previous task has finished

Question

I've written these two wrappers around Tasks, to help with common scenarios in our code where methods are called repeatedly, even if the previous operation hasn't completed yet. There are two common scenarios:

1. An operation is triggered several times by the user (e.g. clicking a checkbox filters a list by its value. User clicks 3 checkboxes in quick succession). In this case, every operation starts a possibly expensive server call, but I only care about the last one.
2. A periodic operation is triggered by a timer every x seconds, but the operation sometimes takes more than x seconds. In this case, I don't want to start a new operation but to simply skip it, waiting for the next timer event to try again.

The solution I've used for both of these is similar:

public static void ExecuteAndCancelPreviousTask(ref Task previousTask, ref CancellationTokenSource cancellationTokenSource, Action action)
{
    if (previousTask != null && !previousTask.IsCompleted)
        cancellationTokenSource.Cancel();

    cancellationTokenSource = new CancellationTokenSource();
    previousTask = Task.Run(action, cancellationTokenSource.Token);
}

public static void ExecuteIfPreviousTaskComplete(ref Task previousTask, ref CancellationTokenSource cancellationTokenSource, Action action)
{
    if (previousTask != null && !previousTask.IsCompleted)
        return;

    cancellationTokenSource = new CancellationTokenSource();
    previousTask = Task.Run(action, cancellationTokenSource.Token);
}

In both cases, I receive the previously running operation (which can be null, the first time) and the CancellationTokenSource attached to it. In the first case, I use the CTS to cancel the previous operation. In the second, I simply ignore it and skip running the action.

Of course, the Action passed has to be CTS-aware, and check it for cancellation:

public void Timer_Elapsed()
{
    TaskExtensions.ExecuteIfPreviousTaskComplete(ref _pollingTask, ref _pollingCancellationTokenSource, async () => 
    {
        var cts = _pollingCancellationTokenSource;
        var pollingResults = await _service.Poll();
        if (cts.IsCancellationRequested)
           return;
        UpdateFromResults(pollingResults);    
    });
}     

My questions are:

1. Do you feel that these methods clearly express their purpose and usage?

2. I'm a bit leery of the two ref parameters. But since the methods have the update the task and its CTS, I could see no better way.

3. I don't really have a need for the action to be run on a new thread. I mainly use Task.Run() because I need, well, a Task that can be monitored and cancelled. Should I instantiate the task some other way?

4. The two methods are almost identical except for the second line of each, but I figured that their usage and intent are clearer when they're separate. Should I merge them? Should I keep them as two distinct public methods, but merge their implementation into a single private method? Or is this overkill for two and a half lines of code?

5. In the first case, should I wait, after Canceling the task, before creating a new one? Make sure it's ended?

6. Currently, I'm capturing the CancellationTokenSource at the beginning of my Action so that it doesn't get replaced when the ref is updated. Is this necessary? Or will the capture keep the original value?

Answer 1

I think you might by overengineering it. Have you tried the Task.ContinueWith method which

Creates a continuation that executes asynchronously when the target Task completes.

There are multiple overloads that let you specify a lot of options with the TaskContinuationOptions enumeration.

Answer 2

Do you feel that these methods clearly express their purpose and usage?

• ExecuteIfPreviousTaskComplete <- yes
• ExecuteAndCancelPreviousTask <- no because it does not do what its name say.

I'm a bit leery of the two ref parameters. But since the methods have the update the task and its CTS, I could see no better way.

Yes, using 2 refs that way is nothing I would do. I think the need of the 2 ref cursors uncovers that there is anything wrong with the design. Passing instance variables as ref parameters to utility methods should also be avoided.

Currently, I'm capturing the CancellationTokenSource at the beginning of my Action so that it doesn't get replaced when the ref is updated. Is this necessary? Or will the capture keep the original value?

Yes, that is necessary and another indication of the complexity of the code. You actually don't know if an instance variable is reinitialized while a method within the class is running...

The two methods are almost identical except for the second line of each, but I figured that their usage and intent are clearer when they're separate. Should I merge them? Should I keep them as two distinct public methods, but merge their implementation into a single private method? Or is this overkill for two and a half lines of code?

IMHO it is overkill for two and half line.

---

To your first use case:

Even if you cancel the task, it is very likely that the server has been requested anyway. IMHO, a more appropriated approach is to wait for the server and run the last action again if there is one. The following CR question addresses exactly the same problem: Processing input in background while user is typing

To your second use case:

Just stop the timer before running the action and restart it afterwards. That's much simpler and takes less resources.