When I use threads to print things to the console, why does it produce odd results?

Question

I've been getting into Threads lately while reading the very nice pdf from Rob Miles (here). He had an example on page 160 (2012, C# pdf) but it didn't write to the console just did empty loops.

I wrote a very simple thread producing loop that creates 10 threads that are writing their IDs to the screen on each multiple of 1000. This was nice - it showed me how threads were running together. My Questions begins with why is my output so confused? Often when I run the program below, I get multiple "Thread 3 Finished" lines where I'm pretty certain, I should only have one of each.

I added a "lock" from MSDN to the loop but it still seems to produce odd output (I'll put an example below).

    namespace ConsoleApplication1
    {
        class Program
        {
            static object sync = new object();

            static void Main(string[] args)
            {
                for (int i = 0; i < 10; i++)
                {
                    Thread myThread = new Thread(() => DoThis(i));
                    myThread.Start();
                }
                Console.ReadLine();
            }

            static void DoThis(int s)
            {
                lock (sync) // a new addition that hasn't helped
                {
                    for (long j = 0; j < 10000; j++)
                    {
                        if (j % 1000 == 0) Console.Write(String.Format("{0}.", s));
                    }
                    Console.WriteLine("\r\nThread {0} Finished", s);
                    Debug.Print("\r\nThread {0} Finished", s); //<-- added to debug

                }
            }
        }
    }

I thought I was doing okay - I have local variables (my counting loop), I have an int that is passed presumably not by reference and later I tried to lock it while doing it's loop. No joy.What would I need to do to make the output look sensible? I tried Deubg.Print to troubleshoot but it has errors too (below).

Eventually, I want to use threading in a larger application but if I can't get it right here, I'm not sure I want to!

Example Output from the debug.print line at the end: (note the multiples) ...

Thread 1 Done
The thread '<No Name>' (0x15cc) has exited with code 0 (0x0).

Thread 9 Done
The thread '<No Name>' (0x1d0c) has exited with code 0 (0x0).

Thread 6 Done
The thread '<No Name>' (0x2248) has exited with code 0 (0x0).

Thread 10 Done
The thread '<No Name>' (0x22bc) has exited with code 0 (0x0).

Thread 9 Done
The thread '<No Name>' (0x85c) has exited with code 0 (0x0).

Thread 9 Done
The thread '<No Name>' (0x1628) has exited with code 0 (0x0).

Thread 3 Done
The thread '<No Name>' (0x2384) has exited with code 0 (0x0).

Thread 6 Done

Thread 2 Done

Thread 4 Done
The thread '<No Name>' (0x2348) has exited with code 0 (0x0).
The thread '<No Name>' (0x2420) has exited with code 0 (0x0).
The thread '<No Name>' (0x1ea8) has exited with code 0 (0x0).

Let me know if I can offer any more info on what I've tried.

Answer 1

Your problem here is that you are using a "modified closure" on the loop variable.

Although this has been fixed for foreach loops, for loops still have the issue (and always will)

To fix it, change your Main() to this:

static void Main(string[] args)
{
    for (int i = 0; i < 10; i++)
    {
        int copy = i; // Make a copy of i to prevent the "modified closure" problem.
        Thread myThread = new Thread(() => DoThis(copy));
        myThread.Start();
    }
    Console.ReadLine();
}

See here for more details:

Access to Modified Closure

Eric Lippert's Article on Closures

Answer 2

Try using Parallel.For instead of starting threads yourself in a loop:

Parallel.For(0, 10, DoThis);

If you needed to pass an extra argument (say w) as part of that, then you can do this:

var w = 4;
Parallel.For(0, 10, i => DoThis(i, w));

The other thing to consider of course is that the Console object is exclusive. If you use it from one thread, any other threads that try to use it will be blocked until it's finished with.

Also your lock (sync) is going to stop any two threads from doing their operation at the same time.

Bear in mind that your threads are not guaranteed to execute in any particular order.