Threads carry duplicate string objects

Question

Here is my code. The log file which is created in the startLog method has proper count from 1 to 1000 but log file being created by log method has duplicate values. For example if 88 is repeated then 89 is missed and next number printed is 90. Can any one please explain why is this happening ?

          namespace TestThreading
            {
                public partial class Form1 : Form
                {
                    NLog.Logger logger = NLog.LogManager.GetCurrentClassLogger();
                    public Form1()
                    {
                        InitializeComponent();
                    }

                    private void Form1_Load(object sender, EventArgs e)
                    {
                    }

                    private void button1_Click(object sender, EventArgs e)
                    {
                        new Thread(startLog).Start();
                    }
                    private void startLog()
                    {
                        int i = 0;
                        string str;
                        while (i < 1000)
                        {
                            str = (++i).ToString();
                            logger.Trace(str);
                            Thread t = new Thread(() => log(new TestObject(str)));
                            t.Start();
                        }
                        MessageBox.Show("Done");
                    }
                    private void log(TestObject obj)
                    {
                        logger.Debug(obj.getCount());
                    }
                }

                public class TestObject
                {
                    string count;
                    public TestObject(string i)
                    {
                        count = i;
                    }
                    public string getCount()
                    {
                        return count;
                    }
                }
            }

Good thing about java that it does not allow this at all. Variable must be final or effectively final. — Kulwinderjit, Mar 10 '18 at 06:29

score 2 · Accepted Answer · answered Oct 15 '13 at 20:45

You're closing over a variable (str) when creating your new thread and then mutating that same variable before the code can run.

Remember that closures close over variables not over *values. By the time your anonymous method determines what to pass to log str has already been altered to be another value.

The solution is simple. Just define str inside of the loop so that each closure has its own variable to close over:

while (i < 1000)
{
    string str = (++i).ToString();
    logger.Trace(str);
    Thread t = new Thread(() => log(new TestObject(str)));
    t.Start();
}

Now that variable is never being changed after it's closed over; instead a new str variable is created for each closure and each one has their own.

@TrevorElliott - If you'd like to clean up the duplicates, please search for one and vote to close this question. — mbeckish, Oct 15 '13 at 21:08

score 0 · Answer 2 · answered Oct 15 '13 at 20:49

As Servy already answered, it is because the str variable is defined outside the closure

If this is needed, you can also use ParameterizedThreadStart and pass the variable modified within the loop. This will actually work

    static void Main(string[] args)
    {
        int i = 0;
        string str;
        while (i < 1000)
        {
            str = (++i).ToString();
            Thread t = new Thread((s) => Console.WriteLine(s));
            t.Start(str);
        }

        Console.ReadLine();
    }

Threads carry duplicate string objects

2 Answers2