Threads problems with return value

Question

I'm trying to start one thread, receive string from it and then start n-counts of threads with the received string. Code:

private void button2_Click(object sender, EventArgs e)
{
    string post = null;
    sync = new ManualResetEvent(false);
    var thr = new Thread[1];
    thr[0] = new Thread(delegate() { post = create_note(); });
    thr[0].IsBackground = true;
    thr[0].Start();
    sync.WaitOne();
    decimal value = Program.Data.numericUpDown1;
    int i = 0;
    int j = (int)(value);
    thr = new Thread[j];
    for (; i < j; i++)
    {
       thr[i] = new Thread(() => invite(post)); 
       thr[i].IsBackground = true;
       thr[i].Start();
     }
}

public string create_note()
{
    while (true)
    {
        string acc = "";
        string proxy = "";
        if (Program.Data.checkBox1 || Program.Data.checkBox2)
        {
            if (Program.Data.checkBox1)
                Proxy.type = "http";
            else if (Program.Data.checkBox2)
                Proxy.type = "socks5";
            lock (locker)
            {
                if (Proxy.proxies.Count == 0)
                {
                    foreach (string prox in File.ReadAllLines(proxy_path))
                    {
                        if (prox.Contains(":"))
                            Proxy.proxies.Add(prox);
                    }
                }
            }
            proxy = rand_proxy();
        }
        else if (!Program.Data.checkBox1 && !Program.Data.checkBox2)
            Proxy.type = "none";
        if (edit_accs.Count == 0)
        {
            break;
        }
        else
            acc = edit_accs.Dequeue();
        Od_post od_post = new Od_post(acc, proxy, Proxy.type);
        string login = od_post.Auth();

        if ()
        {
            string url = rand_url();
            var text = new RandomString(Program.Data.textBox3).ToString();

            string wall_post_text = od_post.wall_post_text(get_text(text), url);
            if (wall_post_text == "Good")
            {

                string image_add = od_post.image_add(post_image_path);
                if (image_add.Split('|')[0] == "Good")
                {
                    if (Program.Data.checkBox5)
                    {
                        string change_name = od_post.change_name();
                        if (change_name == "Changed")
                        {                                        
                        }
                        else
                        {
                        } 
                    }
                    sync.Set();
                    return image_add.Split('|')[1];
                }
                else
                { 
                }
            }
            else
            {
            }
        }
        else
        {
            lock (locker)
            {
                accs.Enqueue(acc);
                Proxy.proxies.Remove(proxy);
            }  
        }
    }
    return "Failed";
}

But it doesn't work. My app hangs, and the post doesn't receive the return value from create_note(). Why?

Answer 1

thr[0].Start();
sync.WaitOne();

This is a pretty common mistake when using a thread. Threading can give you two benefits. One is that code can run concurrently, allowing you to get more work done if you have a machine with multiple cores. And more commonly, it can run code asynchronously, a consideration when you have a user interface that you want to keep responsive.

You are getting neither. It does not run concurrently, your main thread isn't doing any work since it is waiting for the thread to complete. And it doesn't run asynchronously either, your main thread is frozen while the thread is doing its job.

You instead get all the disadvantages of a thread. A classic threading bug is a "race", your code has one. You hope that the "post" variable is assigned after the WaitOne() method completes. It might be, but the odds are just not that good. Since you call Set() before the assignment executes. It can only work correctly when you call Set() after the assignment. Deadlock is another classic threading bug, I don't see one but your debugger can easily show you. Deadlock is very likely here because you are freezing your main thread. Making calls like Invoke() on the thread is going to deadlock.

There's a very simple substitute for your code that does everything that you hope your current code does, minus the threading bugs:

string post = create_note();

Problem solved.

Minus addressing the reason that you considered writing this code in the first place. Which does require that you move all of the code that's now after the WaitOne() call into a callback that runs when the thread completes. Such code tends to be difficult to write, dealing with asynchronicity isn't that easy. But you can get help from .NET to get this right, like the BackgroundWorker.RunWorkerCompleted event, the Task class with its TaskScheduler.FromCurrentSynchronizationContext() method. And the async keyword added to C# version 5.