How to close all tasks if at least one is over

user_7882862 0
Viewed 78

There is a decimal parameter. Suppose it is equal to 100. There is a Task that reduces it by 0.1 every 100ms. As soon as the parameter becomes equal to 1, the task should end and the parameter should not decrease any more. Works without problems if there is only one Task. But if there are 2, 3, 100... then the parameter will eventually become less than 1. I try to use CancellationToken to end all tasks, but the result is still the same. My code:

    class Program
    {
        static decimal param = 100;
        static CancellationTokenSource cancelTokenSource = new CancellationTokenSource();
        static CancellationToken token;

        static void Main(string[] args)
        {
            int tasksCount = 16;
            token = cancelTokenSource.Token;

            Console.WriteLine("Start Param = {0}", param);
            Console.WriteLine("Tasks Count = {0}", tasksCount);

            var tasksList = new List<Task>();
            for (var i = 0; i < tasksCount; i++)
            {
                Task task = new Task(Decrementation, token);
                tasksList.Add(task);
            }
            tasksList.ForEach(x => x.Start());
            Task.WaitAny(tasksList.ToArray());

            Console.WriteLine("Result = {0}", param);
            Console.Read();
        }

        private static void Decrementation()
        {
            while (true)
            {
                if (token.IsCancellationRequested)
                {
                    break;
                }

                if (CanTakeMore())
                {
                    Task.Delay(100);
                    param = param - 0.1m;
                }
                else
                {
                    cancelTokenSource.Cancel();
                    return;
                }
            }
        }

        private static bool CanTakeMore()
        {
            if (param > 1)
            {
                return true;
            }
            else
            {
                return false;
            }
        }
    }

The output is different, but it is always less than 1. How to fix ?

async-await task .net c#
2 Answers

Your tasks are checking and modifying the same shared value in parallel, to some degree as allowed by your CPU architecture and/or Operating System.

Any number of your tasks can encounter a CanTakeMore() result of true "at the same time" (when they call it with the shared value being 1.1m), and then all of them that received true from that call will proceed to decrease the shared value.

This problem can usually be avoided by using a lock statement:

private static object _lockObj = new object();

private static void Decrementation()
{
    while (true)
    {
        if (token.IsCancellationRequested)
        {
            break;
        }
        lock (_lockObj)
        {
            if (CanTakeMore())
            {
                Task.Delay(100); // Note: this needs an `await`, but the method we're in is NOT `async`...!
                param = param - 0.1m;
            }
            else
            {
                cancelTokenSource.Cancel();
                return;
            }
        }
    }
}

Here is a working .NET Fiddle: https://dotnetfiddle.net/BA6tHX

user_12205500

While your code has other issues (such as the fact that you must await Task.Delay), the fundamental problem is that you must either lock your entire read/write operation, or modify your implementation to enable atomic read and writes.

One option is to take your incoming decimal and convert it to a 32 bit integer, multiplying the param by the number of places of precision you need. In this case, it would be 100 * 10 since you have 1 place of precision.

This enables you to use Thread.VolatileRead in conjunction with Interlocked.CompareExchange to produce the behavior you are looking for (working example).

void Main()
{
    int tasksCount = 16;
    token = cancelTokenSource.Token;

    Console.WriteLine("Start Param = {0}", param);
    Console.WriteLine("Tasks Count = {0}", tasksCount);

    var tasksList = new List<Task>();
    for (var i = 0; i < tasksCount; i++)
    {
        Task task = new Task(Decrementation, token);
        tasksList.Add(task);
    }
    tasksList.ForEach(x => x.Start());
    Task.WaitAny(tasksList.ToArray());

    Console.WriteLine("Result = {0}", param);
    Console.Read();
}

static int param = 1000;
static CancellationTokenSource cancelTokenSource = new CancellationTokenSource();
static CancellationToken token;

private static void Decrementation()
{
    while (true)
    {
        if (token.IsCancellationRequested)
        {
            break;
        }

        int temp = Thread.VolatileRead(ref param);
        if (temp == 1)
        {
            cancelTokenSource.Cancel();
            return;
        }
        
        int updatedValue = temp - 1;
        if (Interlocked.CompareExchange(ref param, updatedValue, temp) == temp)
        {
            // the update was successful. Delay (or do additional work)

            // this still does nothing
            // You might want to make your method async or switch to a timer
            Task.Delay(100); 
        }
    }
}

The advantage of Thread.VolatileRead + Interlocked.CompareExchange over a straight lock is that if there is any significant work being done in the lock, this approach will perform significantly better. When benchmarked against the following reasonable locking implementation using decimal subtraction:


private static object _locker = new object();
private static decimal param2 = 100.0m;

private static void DecrementationLock()
{
    while (true)
    {
        if (token.IsCancellationRequested)
        {
            break;
        }

        lock (_locker)
        {
            if (param2 > 1)
            {
                param2 = param2 - 0.1m;
                Task.Delay(100);
            }
            else
            {
                cancelTokenSource.Cancel();
                return;
            }
        }
    }
}

even though the Task.Delay is not awaited in either case, the lock code is over 2.5x slower. That said, in cases where no work is being done, there is essentially no execution time difference between the two approaches.

user_11659980
Related