Locking in C#

c#, concurrency, multithreading, synchronization

Solution

Since none of the code you've written modifies the static field after initialization, there is no need for any locking. Just replacing the string with a new value won't need synchronization either, unless the new value depends on the results of a read of the old value.

Static fields aren't the only things that need synchronization, any shared reference which could be modified is vulnerable to synchronization issues.

class Foo
{
    private int count = 0;
    public void TrySomething()    
    {
        count++;
    }
}

You might suppose that two threads executing the TrySomething method would be fine. But its not.

- Thread A reads the value of count (0) into a register so it can be incremented.

- Context switch! The thread scheduler decides thread A has had enough execution time. Next in line is Thread B.

- Thread B reads the value of count (0) into a register.

- Thread B increments the register.

- Thread B saves the result (1) to count.

- Context switch back to A.

- Thread A reloads the register with the value of count (0) saved on its stack.

- Thread A increments the register.

- Thread A saves the result (1) to count.

So, even though we called count++ twice, the value of count has just gone from 0 to 1. Lets make the code thread-safe:

class Foo
{
    private int count = 0;
    private readonly object sync = new object();
    public void TrySomething()    
    {
        lock(sync)
            count++;
    }
}

Now when Thread A gets interrupted Thread B cannot mess with count because it will hit the lock statement and then block until Thread A has released sync.

By the way, there is an alternative way to make incrementing Int32s and Int64s thread-safe:

class Foo
{
    private int count = 0;
    public void TrySomething()    
    {
        System.Threading.Interlocked.Increment(ref count);
    }
}

Regarding the second part of your question, I think I would just go with whichever is easier to read, any performance difference there will be negligible. Early optimisation is the root of all evil, etc.

Why threading is hard

Problem

I'm still a little unclear and when to wrap a lock around some code. My general rule-of-thumb is to wrap an operation in a lock when it reads or writes to a static variable. But when a static variable is ONLY read (e.g. it's a readonly that is set during type initialization), accessing it doesn't need to be wrapped in a lock statement, right? I recently saw some code that looked like the following example, and it made me think there may be some gaps in my multithreading knowledge: ``` class Foo { private static readonly string bar = "O_o"; private bool TrySomething() { string bar; lock(Foo.objectToLockOn) { bar = Foo.bar; } // Do something with bar } } ``` That just doesn't make sense to me--why would there by concurrency issues with READING a register? Also, this example brings up another question. Is one of these better than the other? (E.g. example two holds the lock for less time?) I suppose I could disassemble the MSIL... ``` class Foo { private static string joke = "yo momma"; private string GetJoke() { lock(Foo.objectToLockOn) { return Foo.joke; } } } ``` vs. ``` class Foo { private static string joke = "yo momma"; private string GetJoke() { string joke; lock(Foo.objectToLockOn) { joke = Foo.joke; } return joke; } } ```

Original source