0

Is it a good practice to have a static variable as counter that can be updated by all the threads in a C#.NET program?
Sample Code:

public class SomeTask 
{
static int count = 0;
public void Process()
{

    while(true)
    {
        //some repeated task
        count++;
        if(count>100000)
        {
            count=0;
            break;
        }
    }
}
}

public class WorkerRole : RoleEntryPoint
{
   public override void Run()
    {
    while(true)
    {

        for (int i = 0; i < maxTasks; i++)
        {
            this.Tasks[i] = Task.Factory.StartNew(() => (new SomeTask()).Process());
        }
        Task.WaitAll(this.Tasks);

        //every 100000 in counter needs some updates at program level
    }
}
}
Srinivas
  • 2,479
  • 8
  • 47
  • 69
  • 2
    Beside the answers already made: This might depend on what you are trying to do, IMHO. – Samuel Dec 17 '13 at 09:34
  • There's no good practices to having a thread-safe "counter" - as with all asynchronous programming the decisions have to be much more informed, based on the problem you're trying to solve. A counter may be safe or it may not, depending on your situation. – Paul Turner Dec 17 '13 at 09:35
  • 2
    If you implement one, use either `lock` or `Interlocked.Increment` (faster) – CodesInChaos Dec 17 '13 at 09:53
  • @Seenu what I meant was not how your code look likes but rather what specific type of task are you trying to solve with this approach as your sample code does not make clear what purpose the Task Array has, as your code seems impractical to me. – Samuel Dec 17 '13 at 13:08

3 Answers3

2

Yes that is a good approach but make sure you use an atomic type. I would have performance concerns if the counter increment operations were implemented using thread safe code as opposed to atomic operations.

To implement a counter, you'll be making use of ++ and --. These, in general, are not thread safe for the primitive types in C#.

See Is the ++ operator thread safe?

Atomic types in C#?

See reference What operations are atomic in C#?

This answer suggests that 32 bit integral types are atomic on 32 bit machines, 64 bit integral types are atomic on 64 bit machines.

Community
  • 1
  • 1
Bathsheba
  • 231,907
  • 34
  • 361
  • 483
2

If you can't avoid it then it's fine. It's best to use Interlocked class increment the counter:

if (Interlocked.Increment(ref counter) % 100000 == 0) {
    // Do something every hundred thousand times
    // Use "== 1" if you also want to do it on the first iteration
}

I'll leave the code below for the case when you only need to know the count at the end

In your case you could keep the count as an instance field (i.e. non-static), add public getter and sum up all the counters after all tasks have finished:

public class SomeTask 
{
    int count = 0;
    public int Count { get { return count; } }

    public void Process()
    {

        while(true)
        {
            //some repeated task
            count++;

            if (something)
                break;
        }
    }        
}

var someTasks = new List<SomeTask>();
for (int i = 0; i < maxTasks; i++)
{
    var someTask = new SomeTask();
    someTasks.Add(someTask);
    this.Tasks[i] = Task.Factory.StartNew(() => someTask.Process());
}
Task.WaitAll(this.Tasks);

// Your total count
var total = someTasks.Sum(t => t.Counter);
Konstantin Spirin
  • 20,609
  • 15
  • 72
  • 90
0

Yes, it is perfectly all right as long as you keep it thread-safe and syncronized in threads. You can use lock for syncronization. You can read more about thread safety and static members here and here. You can also use Interlocked.Increment to increament the counter in a thread-safe way. Read more about thread safety with increament operator here in Eric Lippert answer.

class Account
{
    int count;
    private Object thisLock = new Object();

    public void Add(decimal amount)
    {
        //your code
        lock (thisLock)
        {
           count = count + 1;
        }
    }
}
Community
  • 1
  • 1
Adil
  • 146,340
  • 25
  • 209
  • 204
  • Would it be correct to say that a lock isn't necessary, in the case where the count is only being incremented. Or could an `Exception` occur even in this operation? – tjheslin1 Dec 17 '13 at 09:50
  • 1
    I would advise against using lock in this scenario. Locking can be really expensive and doing locking just to increment a counter can kill lot of performance. – Euphoric Dec 17 '13 at 09:51
  • @Euphoric My thoughts exactly. – tjheslin1 Dec 17 '13 at 10:16
  • I agree with the cost of locking but what if multiple threads tend to increment that member concurrently, we shall have race condition here and that lead to non deterministic behavior. – Michel Hanna Feb 13 '19 at 10:21