Lambda expressions with multithreading in C#

Question

I am trying to understand why this program doesn't work

Expected output: numbers 0-19 in random order What I get when I run: some numbers repeat, sometimes 20 is printed.

Please help. I tried with lock(obj) in DoSomething() but it didn't help.

Program

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;

namespace ConsoleApplication2
{
    public delegate void callbackDelegate(int x);
    class Program
    {
        void processCallback(int x)
        {
            Console.WriteLine("IN callback: The value I got is " + x);
        }

        static void Main(string[] args)
        {
            Program p = new Program();
            p.processinThreads();
            Console.ReadKey();
        }

        public void DoSomething(int x, callbackDelegate callback)
        {
            Thread.Sleep(1000);
            //Console.WriteLine("I just woke up now " + x);
            callback(x);
        }

        public void processinThreads()
        {
            for (int i = 0; i < 20; i++)
            {
                Thread t = 
new Thread(new ThreadStart(()=>DoSomething(i, processCallback)));
                t.Start();
            }
        }
    }
}

right, I couldn't get this when I searched for this problem. Well I don't know this 'closure over lambda problem' :) — satya
– satya, Commented Feb 23, 2012 at 12:08
Avoid manual thread creation. Here is a detailed explanation with benchmarks why — Anastasiosyal
– Anastasiosyal, Commented Feb 23, 2012 at 13:49

Jakub Konecki · Accepted Answer · 2012-02-23 12:02:44Z

9

public void processinThreads()
{
    for (int i = 0; i < 20; i++)
    {
        int local = i;
        Thread t = new Thread(new ThreadStart(()=>DoSomething(local, processCallback)));
        t.Start();
    }
}

Your problem is related to closure over lambda.

answered Feb 23, 2012 at 12:02

Jakub Konecki

46.1k8 gold badges92 silver badges128 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Paolo Moretti Over a year ago

Check out this article: Closing over the loop variable considered harmful. It will be fixed in C# 5.0.

Paul Tyng · Accepted Answer · 2012-02-23 12:05:59Z

7

You should just use the TPL, its a lot easier and recommended over manual thread management:

Parallel.For(0, 20, x => {
    Thread.Sleep(1000);
    Console.WriteLine("IN callback: The value I got is " + x);
});

This will also block until the loop finishes, if you don't want that you can use the TPL Task, but I would definitely recommend avoiding threads.

answered Feb 23, 2012 at 12:05

Paul Tyng

7,6051 gold badge36 silver badges57 bronze badges

2 Comments

Matthias Meid Over a year ago

Moreover, this is likely to be more efficient: Spawning another thread is pretty expensive. The TPL uses an (automagically computed) optimum number of threads and re-uses them.

satya Over a year ago

@user676571 I am planning to use TPL with Async pattern applied.

Matthias Meid · Accepted Answer · 2012-02-23 12:07:11Z

1

As Jakub already told you, you need to copy i into another local variable local. In your code, the Delegates have direct access to i itself, not a copy of i, so they print out the very current value of i, which may be greater than when you started the thread. This is called closure.

answered Feb 23, 2012 at 12:07

Matthias Meid

12.5k7 gold badges48 silver badges79 bronze badges

Collectives™ on Stack Overflow

Lambda expressions with multithreading in C#

Program

3 Answers 3

1 Comment

2 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

Program

3 Answers 3

1 Comment

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related