8

I have a class like the one below:

class Program
    {
        static void Main(string[] args)
        {
            var outputWindow = new OutputWindow();

            var threads = new List<Thread>();

            Action action = () => outputWindow.Display(20);

            for (int i = 0; i < 10; i++)
            {
                var thread = new Thread(() => action()) {Name = "Thread " + i};
                threads.Add(thread);
            }

            foreach (var thread in threads)
            {
                thread.Start();
            }
        }
    }

    public class OutputWindow
    {
        public void Display(int x)
        {
            for (int i = 0; i < x; i++)
            {
                Console.WriteLine(Thread.CurrentThread.Name + " Outputcounter: " + i);
            }
        }
    }

Question is- is this thread safe and will this lead to any race condition on the local variable i inside the display method? Will all the threads increment the value of the variable "i" as expected(which is it increments the value and does not encroch into other threads value of i)

If this is threadsafe, will it be safe to assume that any local variables declared within a method is always thread safe and shared variables are the ones which needs synchronization?

Thanks, -Mike

H H
  • 263,252
  • 30
  • 330
  • 514
Mike
  • 3,204
  • 8
  • 47
  • 74
  • You have a local variable (not parameter) `action` (which does capture in a lambda another local variable). You then launch several threads, giving each thread a new lambda which captures the same `action` local variable. That `action` variable is really converted to a field. Surely, it is not so "local" after all. So those local variables are not thread-safe. When you ask about the variable `i` in your code, it is truely local. It is declared inside a method, it is not captured by any lambdas, it is not passed with `ref` or `out`, it is an immutable type. So it is clearly not problematic. – Jeppe Stig Nielsen Jan 17 '14 at 11:28
  • 1
    Sorry, I was talking about the `i` in the `Display` method. The `i` in the `Main` method ***is*** captured in a lambda. And that is clearly problematic. The `i` is turned into a field which will only occur in _one_ instance. You probably want this: `for (int i = 0; i < 10; i++) { int copyOfI = i; var thread = new Thread(() => action()) {Name = "Thread " + copyOfI}; threads.Add(thread); }`. This was noted in Henk's answer also. With that, `copyOfI` will be turned into an instance field on a generated class, and there will be _ten_ instances of that class. – Jeppe Stig Nielsen Jan 17 '14 at 11:34
  • My previous comment was not correct after all, I think. It is fine to say `new Thread(() => action()) /* ANONYMOUS LAMBDA HAS STOPPED HERE */ {Name = "Thread " + i /* Not capturing */ };`. – Jeppe Stig Nielsen Jan 17 '14 at 12:03

3 Answers3

16

Each method invocation will have a separate set of local variables. However, those variables could refer to objects which other methods also use. For example:

public void AppendSomething(StringBuilder builder)
{
    builder.Append("Something");
}

Here builder is still a local variable (parameters are local variables) and each invocation of AppendSomething will have an independent variable - but you could call the method from multiple threads using the same StringBuilder reference, so that method is not thread-safe. (As StringBuilder isn't thread-safe.)

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
4

As long as your data is local, yes, it's thread-safe. If it references data outside of that method, then it's not.

In your example, i is not affected by multiple threads (because you create the variable, set a local value and just increment the value).

If i was referencing something outside of the method (for example a file) then it wouldn't be thread-safe, because you could be referring to the same file on different threads.

Kenneth
  • 28,294
  • 6
  • 61
  • 84
1

Local variables are thread safe within a method as long as you don't pass them to other thread. If you do, then it depends.

For example, in your code all the threads you create have shared access to outputWindow and threads. If the objects are not thread safe and you call them from multiple threads then you may experience problems. For example, if each thread were to try and remove itself from threads when it finishes then you'd have a problem as List<> isn't thread safe for read/write.

All your threads are using the same OutputWindow instance. If the call to Display mutated the state of the object then need ensure that mutation was threadsafe, otherwise you'd have a race condition.

Sean
  • 60,939
  • 11
  • 97
  • 136