Multithreaded problems with lock

Question

Hi guys I have such construction: Start threads:

 Thread[] thr;
    int good_auth, bad_auth, good_like, bad_like;
    static object accslocker = new object();
    static object limitlocker = new object();
    string acc_path, proxy_path, posts_path;
    int position_of_limit, position = 0;
    private void button1_Click(object sender, EventArgs e)
    {
        button1.Enabled = false;
        button2.Enabled = true;
        decimal value = numericUpDown1.Value;
        int i = 0;
        int j = (int)(value);
        thr = new Thread[j];
        for (; i < j; i++)
        {
            thr[i] = new Thread(new ThreadStart(go));
            thr[i].IsBackground = true;
            thr[i].Start();
        }
    }

And than function go:

    public void go()
    {
        while (true)
        {
            string acc = "";
            string proxy = "";

            lock (limitlocker)
            {
                if (position_of_limit >= int.Parse(textBox2.Text) - 1)
                {
                    position_of_limit = 0;
                    if (position < posts.Count - 1)
                        position++;
                    else
                    {
                        break;
                    }
                }

            } 
            lock (accslocker)
            {
                if (accs.Count == 0)
                {
                    break;
                }
                else
                    acc = accs.Dequeue();
            }
            OD od = new OD(acc);
            string login = od.Auth();
            if (login == "Login")
            {
                lock (accslocker)
                {
                    good_auth++;
                    log_good_auth(good_auth);
                }

                string like = od.like(posts[position], textBox1.Text);
                if (like == "Good")
                {
                    lock (accslocker)
                    {
                        position_of_limit++;
                        good_like++;
                        log_good_like(good_like);
                    }
                }
                else if (like == "Failed")
                {
                    lock (accslocker)
                    {
                        bad_like++;
                        log_bad_like(bad_like);
                    }
                }
                else
                {
                    lock (accslocker)
                    {
                        bad_like++;
                        log_bad_like(bad_like);
                    }
                }

            }
            else if (login == "Spamblock")
            {
                lock (accslocker)
                {
                    bad_auth++;
                    log_bad_auth(bad_auth);
                }
            }
            else if (login == "Locked")
            {
                lock (accslocker)
                {
                    bad_auth++;
                    log_bad_auth(bad_auth);
                }
            }
            else if (login == "Invalid")
            {
                lock (accslocker)
                {
                    bad_auth++;
                    log_bad_auth(bad_auth);
                }
            }
            else if (login == "Bad_proxy")
            {
                lock (accslocker)
                {
                    accs.Enqueue(acc);
                    Proxy.proxies.Remove(proxy); 
                }
            }
            else
            {
                lock (accslocker)
                {
                    accs.Enqueue(acc);
                    Proxy.proxies.Remove(proxy);
                }
            }
        }
    }

I start for example 20 threads, when position_of_limit becomes bigger than int.Parse(textBox2.Text) - 1 all threads need to take next posts[position] in next loop. But I receive an exception on line string like = od.like(posts[position], textBox1.Text);

"Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index"

How to solve this problem? Thanks

Use a debugger and determine the root cause by tracing control and data flow. — usr, Jul 10 '13 at 20:35
At a glance this looks like a nightmare to debug with all those locks in a single method. — Brian Rasmussen, Jul 10 '13 at 20:39
The most likely line is "string like = od.like(posts[position], textBox1.Text);". What value does position have, and how big is posts? Get the stack trace and find where it occurs, or use a debugger to figure it out. — user1676075, Jul 10 '13 at 20:40
@BrianRasmussen Definitely a nightmare to debug... where is position defined? — Kevin, Jul 10 '13 at 20:40
Wow, you have a lot of repeated code in there. When you copy-paste a lot or otherwise exactly repeat code you might want to consider moving that code into a method. It's also possible to combine a lot of your if/else blocks together using boolean operators. If two else if(){} blocks contain the same code, then just combine the conditions using || and use a single block! — Odrade, Jul 10 '13 at 20:44
It looks like after you have incremented position - `if (position < posts.Count - 1) position++;` - some other thread removes one or more entries from `posts` before you reach `od.like(posts[position], textBox1.Text);` - which means its race condition and you are not locking properly. You have not shown what creates and deletes entries from `posts`. — YK1, Jul 10 '13 at 21:26

score 0 · Answer 1 · edited May 23 '17 at 11:49

0

Locking the GUI thread is evil, since it's an STA thread. That means it must manage a message loop and is not allowed to block. So blocking is likely to cause deadlocks.

Rather use callbacks like BackgroundWorker.RunWorkerCompleted.

Some other informative links about locking:

edited May 23 '17 at 11:49

Community

1
1

answered Jul 10 '13 at 20:59

Fabian Bigler

10,403
6
47
70

1

I'm confused. Where do you get STA from: `new Thread().Start()`? – Jul 10 '13 at 21:17
1

I can prove he is locking on the UI thread, there is a `textBox2.Text` (line 10 of `go`) inside a `lock` if he was not on the UI thread he would get a cross thread exception. – Scott Chamberlain Jul 10 '13 at 21:28
@ScottChamberlain: Although its not recommended - if you _read_ textbox.Text from another thread, WinForms app somehow does not complain and does not throw cross thread exception. – YK1 Jul 10 '13 at 21:51
@ScottChamberlain: Also wanted to mention that this, however, is fixed in WPF. So, in my opinion, `go()` is not running on UI thread. – YK1 Jul 10 '13 at 22:04
@YK1 That I did not know, I appologize. – Scott Chamberlain Jul 10 '13 at 22:22

Multithreaded problems with lock

1 Answers1