Async Multiple Tasks Threading Issue

Question

I am attempting to execute multiple (n) tasks asynchronously which aggregate to a single summed result. Currently I have the following:

public class Foo
{
    public async void DoWork()
    {
        var stopwatch = Stopwatch.StartNew();
        List<Task> tasks = new List<Task>();

        var bar = new Bar();

        for (int i = 0; i < 20; i++)
        {
            var task = Task.Run(() =>
            {
                Thread.Sleep(500);

                bar.Count1++;
                bar.Count2++;
            });

            tasks.Add(task);
        }

        await Task.WhenAll(tasks.ToArray());

        Console.WriteLine("All done, Bar Count1: " + bar.Count1 + ", Count2: " + bar.Count2);
        stopwatch.Stop();
        Console.WriteLine("Time taken " + stopwatch.ElapsedMilliseconds + " ms");
    }
}

public class Bar
{
    public int Count1 { get; set; }
    public int Count2 { get; set; }
}

I would expect bar.Count1 and bar.Count2 to have values of 20 at the end of execution here, however, each time I run the program I get different values for each of them (which are most of the time < 20). How do I get around this problem?

If your tasks are truly CPU-bound, then I recommend using Parallel or PLINQ, both of which have aggregation support built-in. — Stephen Cleary
– Stephen Cleary, Commented May 20, 2014 at 23:42

Community · Accepted Answer · 2017-05-23 11:56:10Z

4

Nothing closure problem here. ++ operator is not thread safe. So you need a lock around it.

lock(bar)
{    
    bar.Count1++;
    bar.Count2++;    
}

That should solve the problem.

edited May 23, 2017 at 11:56

CommunityBot

11 silver badge

answered May 19, 2014 at 4:04

Sriram Sakthivel

73.8k7 gold badges118 silver badges200 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Scott Chamberlain Over a year ago

Had these been fields instead of properties you also could have used Interlocked.Increment( and not need to lock (just in case the OP's real case involved fields instead of properties).

Sriram Sakthivel Over a year ago

@ScottChamberlain True, but OP needs it as public. Public fields is not a good option. One way is to have increment logic inside Bar class itself.

Collectives™ on Stack Overflow

Async Multiple Tasks Threading Issue

1 Answer 1

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related