Threads cause GUI to freeze up

Question

So I'm not the most experienced with the C# programming language, however I've been making a few test applications here and there.

I've noticed that the more threads I create for an application I'm working on, the more my GUI starts to freeze. I'm not sure why this occurs, I previously thought that part of the point of multi-threading an application was to avoid the GUI from freezing up.

An explanation would be appreciated.

Also, here's the code I use to create the threads:

private void runThreads(int amount, ThreadStart address)
{
    for (int i = 0; i < amount; i++)
    {
        threadAmount += 1;
        Thread currentThread = new Thread(address);
        currentThread.Start();
    }
}

and here's what the threads run:

private void checkProxies()
{
    while (started)
    {
        try
        {
            WebRequest request = WebRequest.Create("http://google.co.nz/");
            request.Timeout = (int)timeoutCounter.Value * 1000;
            request.Proxy = new WebProxy(proxies[proxyIndex]);
            Thread.SetData(Thread.GetNamedDataSlot("currentProxy"), proxies[proxyIndex]);
            if (proxyIndex != proxies.Length)
            {
                proxyIndex += 1;
            }
            else
            {
                started = false;
            }
            request.GetResponse();
            workingProxies += 1;
        }
        catch (WebException)
        {
            deadProxies += 1;
        }

        lock ("threadAmount")
        {
            if (threadAmount > proxies.Length - proxyIndex)
            {
                threadAmount -= 1;
                break;
            }
        }
    }
}

Answer 1

While I cannot tell you why exactly your code is slowing the GUI down, there are a few things in your code you should do to make it all-round better. If the problem persist then, then it should be a lot easier to pinpoint the issue.

1. Creating Thread objects is expensive. This is why new classes were added in C# to better handle multi-threading. Now you have access to the Task class or the Parallel class (described below).
2. Judging from the comments, you're running a LOT of threads at the same time. While it shouldn't be an issue to just run them, you're not really getting much use out of them if what you're doing is firing WebRequests (unless you have an awesome network). Use multiple threads, sure, but limit their number.
3. A Task is great when you want to do a specific operation in the background. But when you want to repeat a single operation for a specific set of data in the background... why not use the System.Threading.Tasks.Parallel class? Specifically, Parallel.ForEach (where you can specify your list of proxies as the parameter). This method also lets you set up how many threads are supposed to run concurrently at any given moment by using ParallelOptions.
4. One more way to code would be to make use of the async and await keywords available in .NET 4.5. In this case your GUI (button press?) should invoke an async method.
5. Use thread-safe methods like Interlocked.Increment or Interlocked.Add to increase / decrease counters available from multiple threads. Also, consider that you could change your list of proxies to a ConcurrentDictionary<string, bool> (where bool indicates if the proxy works or not) and set the values without any worry as each thread will only access its own entry in the dictionary. You can easily queue the totals at the end using LINQ: dictionary.Where(q => q.Value).Count() to get the number of working proxies, for example. Of course other classes are also available, depending on how you want to tackle the issue - perhaps a Queue (or ConcurrentQueue)?
6. Your lock shouldn't really work... as in, it seems it's working by accident than by design in your code (thanks Luaan for the comment). But you really shouldn't do that. Consult the MSDN documentation on lock to get a better understanding of how it works. The Object created in the MSDN example isn't just for show.
7. You can also make the requests themselves multi-threaded by using BeginGetResponse and EndGetResponse methods. You can, in fact, combine this with the Task class for a much cleaner code (the Task class can convert Begin/End method pairs into a single Task object).

So, a quick recap - use the Parallel class for multi-threading, and use concurrency classes for keeping things in place.

Here's a quick example I wrote:

    private ConcurrentDictionary<string, bool?> values = new ConcurrentDictionary<string, bool?>();

    private async void Button_Click(object sender, RoutedEventArgs e)
    {
        var result = await CheckProxies();
        label.Content = result.ToString();
    }

    async Task<int> CheckProxies()
    {
        //I don't actually HAVE a list of proxies, so I make up some data
        for (int i = 0; i < 1000; i++)
            values[Guid.NewGuid().ToString()] = null;
        await Task.Factory.StartNew(() => Parallel.ForEach(values, new ParallelOptions() { MaxDegreeOfParallelism = 10 }, this.PeformOperation));
        //note that with maxDegreeOfParallelism set to a high value (like 1000)
        //then I'll get a TON of failed requests simply because I'm overloading the network
        //either that or google thinks I'm DDOSing them... >_<
        return values.Where(v => v.Value == true).Count();
    }

    void PeformOperation(KeyValuePair<string, bool?> kvp)
    {
        try
        {
            WebRequest request = WebRequest.Create("http://google.co.nz/");
            request.Timeout = 100;
            //I'm not actually setting up the proxy from kvp,
            //because it's populated with bogus data
            request.GetResponse();

            values[kvp.Key] = true;
        }
        catch (WebException)
        {
            values[kvp.Key] = false;
        }
    }

Answer 2

Despite the other comments being correct in that you should use either the Task class, or better yet, the async API, that is not the reason you're locking up.

The line of code that is causing your threads to lock up is this:

request.Timeout = (int)timeoutCounter.Value * 1000;

I am assuming that timeoutCounter is a control on the WinForm - which is running on the main GUI thread.
In other words, your threads' code is trying to access a control which is not in it's own thread, which is not really "allowed", at least not so simply.

For example, this question shows how to do this, albeit most of the answers there are a bit dated.

From a quick google (okay who am I kidding, I binged it) I found this article that explains the problem rather well.