Mends.One

2 threads increasing a static integer

C#

if i am increasing a static by 2 different Tasks or Threads do i need to lock it?

i have this class which will be used by multiple threads at the same time, it returns a proxy to be used inside a thread but i dont want to use the same proxy at the same time with each thread, so i thought incrementing a static integer is the best way, any suggestions?

class ProxyManager
{
    //static variabl gets increased every Time GetProxy() gets called
    private static int Selectionindex;
    //list of proxies
    public static readonly string[] proxies = {};
    //returns a web proxy
    public static WebProxy GetProxy()
    {
      Selectionindex = Selectionindex < proxies.Count()?Selectionindex++:0;
      return new WebProxy(proxies[Selectionindex]) { Credentials = new NetworkCredential("xxx", "xxx") };
    }
}

based on the selected answer

if(Interlocked.Read(ref Selectionindex) < proxies.Count())
{
    Interlocked.Increment(ref Selectionindex);
}
else
{
    Interlocked.Exchange(ref Selectionindex, 0);
}

Selectionindex = Interlocked.Read(ref Selectionindex);
4
U
user1590636
Jump to: Answer 1

Answers (1)

If you increment a static variable across threads you will end up with inconsistent results. Instead use Interlocked.Increment:

private void IncrementSelectionIndex() {
    Interlocked.Increment(ref Selectionindex);
}

64-bit reads are atomic, but to fully support 32-bit systems you should use Interlocked.Read

private void RetrieveSelectionIndex() {
    Interlocked.Read(ref Selectionindex);
}
6
C
cfeduke

Comments:

Guillaume said:
Interlocked.Add should be also used to set the variable back to 0: Interlock.Add(SelectionIndex, -SelectionIndex);
cfeduke said:
Yes, or Interlocked.Exchange(ref Selectionindex, 0)... if you use -SelectionIndex you'd have to use Interlocked.Read to be safe on 32-bit systems and even then... could still create a problem unless you placed the Interlocked.Add inside a locked block locking on an arbitrary object.
user1590636 said:
@cfeduke please check the updated code in my question, is it alright?
cfeduke said:
Appears to be. I'd encapsulate the usage of Selectionindex into its own class (singleton or static, named something like AtomicCount) with methods like Get (Interlocked.Read), Reset (Interlocked.Exchange) and a bool IsLessThan(Int32) to replace the if conditional's test. I would do this for maintainability and readability.

Related Questions