Search code examples
c#multithreadingsystem.timers.timer

Again on using interlocked


I read a lot of answers on this point but I did not found the solution yet.

I have a class with a counter attribute having a problem with cached values. Even volatile don't seems to work:

public class MyClass {
    private Timer _timer;
    private int _threadsCounter = 0;
    public StreamWriter Tracer { get; set; }

    public MyClass() {
        _timer = new Timer(1000.0 * 10);
        _timer.AutoReset = true;
        _timer.Elapsed += new ElapsedEventHandler(OnTimer);
        _timer.Start();
    }

    private void OnTimer(object sender, ElapsedEventArgs e) {
        HashSet<Task> taskPool = new HashSet<Task>();
        try {
            if (Tracer != null) Tracer.WriteLine("[{0}] onTimer start. Current threads counter is {1}.", DateTime.Now, _threadsCounter);
            if (_threadsCounter >= 10) return;

            // create parallel tasks
            for (int i = 0; i < 8; i++) {
                // limit on the max num of parallel processing but the counter remains unchanged during this timer event!!!
                if (_threadsCounter >= 10) break;

                var timeout = (30 + i * 2);
                var task = Task.Run(() => {
                        var localCounter = System.Threading.Interlocked.Increment(ref _threadsCounter);
                        try {
                            System.Threading.Thread.Sleep(timeout * 1000);
                        }
                        finally {
                            System.Threading.Interlocked.Decrement(ref _threadsCounter);
                        }
                    });
                taskPool.Add(task);
            }

        }
        finally {
            if (Tracer != null) 
                Tracer.WriteLine("[{0}] onTimer end. Created {1} tasks. Current threads counter is {2}.", DateTime.Now, taskPool.Count, _threadsCounter);
        }
    }

Well, it seems that the onTimer caches the _threadsCounter variable as the output is:

[14:10:47] onTimer start. Current threads counter is 0.
[14:10:47] onTimer end. Created 8 tasks. Current threads counter is 0.

[14:10:57] onTimer start. Current threads counter is 8.
[14:10:57] onTimer end. Created 8 tasks. Current threads counter is 8.

[14:11:07] onTimer start. Current threads counter is 16.
[14:11:07] onTimer end. Created 0 tasks. Current threads counter is 16.

[14:11:17] onTimer start. Current threads counter is 15.
[14:11:17] onTimer end. Created 0 tasks. Current threads counter is 15.

[14:11:37] onTimer start. Current threads counter is 4.
[14:11:37] onTimer end. Created 8 tasks. Current threads counter is 4.

[14:11:47] onTimer start. Current threads counter is 8.
[14:11:47] onTimer end. Created 8 tasks. Current threads counter is 8.

[14:11:57] onTimer start. Current threads counter is 16.
[14:11:57] onTimer end. Created 0 tasks. Current threads counter is 16.

Why do I arrive to 16? I solved the problem by changing a bit the code:

var localCounter = _threadsCounter;
...
if ((localCounter + taskPool.Count) >= 10) break;

But why this behavior?


Solution

  • Task.Run doesn't start the task immediately. It adds the task to the thread pool queue and returns.

    In your case, the whole for loop is executed before the new tasks start running, so nothing changes _threadsCounter. That's why volatile doesn't help.