0

Following is the code that I use. The main thread waits for Threadpool threads to execute. I use AutoResetEvent (WaitHandle), but I am really surprised that I am way off the mark, as the code doesn't behave as expected.

I have two concentric for loops, where Threadpool is in the inner loop and it is expected that for each iteration of the outer loop, all the inner loop values should be processed. Main thread is made to wait using AutoResetEvent WaitOne call just outside inner loop, a static variable which is reset on every iteration of the outerloop to the max value of the inner loop, and is decremented using Interlock in the method call on Threadpool thread is used to call the Set for the AutoResetEvent. However, even when I expect the static variable to show the value 0 after every inner loop, it doesn't. What is the issue in my code and what are better options for me to accomplish the task? In fact, due to the mix up of the values, the main thread doesn't really seem to be waiting for the Threadpool threads.

using System;
using System.Threading;

namespace TestThreads
{
class Program
{
    private static int threadingCounter = 0;
    private static readonly object lockThreads = new Object();
    private AutoResetEvent areSync = new AutoResetEvent(true);

    // <param name="args"></param>
    static void Main(string[] args)
    {
        Program myProgram = new Program();

        try
        {
            try
            {   
                for (int outer = 0; outer < 1000; outer++)
                {
                    threadingCounter = 500;
                    try
                    {
                        for (int inner = 0; inner < 500; inner++)
                        {
                            ThreadPool.QueueUserWorkItem(new
                                 WaitCallback(myProgram.ThreadCall), inner);
                        }
                    }
                    catch (Exception ex)
                    {
                        Console.WriteLine("Exception :: " + ex.Message);
                    }
                    finally
                    {                            
                        myProgram.areSync.WaitOne();
                    }

                    if(threadingCounter != 0)
                        Console.WriteLine("In Loop1, Thread Counter :: " +
                            threadingCounter);
                }
            }
            catch (Exception ex)
            {
                Console.WriteLine("Exception :: " + ex.Message);
            }                
        }
        catch(Exception ex)
        {
            Console.WriteLine("Exception :: " + ex.Message);
        }
        finally
        {
            threadingCounter = 0;

            if (myProgram.areSync != null)
            {
                myProgram.areSync.Dispose();
                myProgram.areSync = null;
            }
        }
    }

    public void ThreadCall(object state)
    {
        try
        {
            int inner = (int)state;
            Thread.Sleep(1);
        }
        catch (Exception ex)
        {
            Console.WriteLine("Exception :: " + ex.Message);
        }
        finally
        {
            Interlocked.Decrement(ref threadingCounter);
            if (threadingCounter <= 0)
                areSync.Set();
        }                
    }        
 }
}
6
  • Your exception handling is damn awful. There's at least one bug visible, you are not using Interlocked.Decrement correctly. You must use its return value and never use the variable directly while it is being decremented. Use CountDownEvent to fall in the pit of success. Jul 12, 2013 at 19:55
  • @HansPassant, your comment is rude and factually inaccurate. Jul 12, 2013 at 21:22
  • Okay, I'm being bullied into rephrasing that. Your exception handling greatly enhances the readability of your code. Accessing a variable that's being modified by other threads is not a problem. Better? Jul 12, 2013 at 21:31
  • @HansPassant, thanks for your comment, needn't be bullied when you were telling the right thing, I know that exception handling is quite bad, but please note I pasted the code where I was adding Try Catch finally from all sides and focus was understanding the issue with thread syncing and ensure that right calls are made, I never though I am adding a clean professional code. However, when you suggest some such thing please provide at least an example of what you consider a good exception handling, as that would help appreciating your point far better, Thanks, Jul 13, 2013 at 3:41
  • @Tyler Jensen thanks for your comment, but as of now HansPassant suggestion works and i am able to correct the code. However in case there's any good practice for Exception handling in the multi threaded schenario then please suggest. Jul 13, 2013 at 3:50

2 Answers 2

2

You have initialized AutoResetEvent with initial state as signaled(true) which will allow first call to

myProgram.areSync.WaitOne();

to continue without blocking, so it continues to outer loop and Queues execution to Threadpool again, hence the results are messed up. it is very clear.

update your code to

private AutoResetEvent areSync = new AutoResetEvent(false);

for expected results. hope this helps

3
  • AutoResetEvent initialization with a value true is a mistake in my post, since I was trying multiple things, I have made it false, that doesn't help. Any other point that may help Jul 12, 2013 at 21:09
  • nope, setting new AutoResetEvent(false) worked for me i tested ur code. Jul 12, 2013 at 21:25
  • above mentioned suggestion by Hans corrects the code, it is not correct otherwise. Will lead to an issue ion multi threaded scenario Jul 13, 2013 at 3:53
0

I would refactor this using something like this. This assumes you want to do something in background threads in your inner loop, finish each of those before proceeding with your next outer loop, avoid messy exception handling while still capturing the exceptions that occurred during processing so that you can deal with those exceptions after processing is complete for both inner and outer loop.

// track exceptions that occurred in loops
class ErrorInfo
{
    public Exception Error { get; set; }
    public int Outer { get; set; }
    public int Inner { get; set; }
}

class Program
{
    static void Main(string[] args)
    {
        // something to store execeptions from inner thread loop
        var errors = new ConcurrentBag<ErrorInfo>();
        // no need to wrap a try around this simple loop
        // unless you want an exception to stop the loop
        for (int outer = 0; outer < 10; outer++)
        {
            var tasks = new Task[50];
            for (int inner = 0; inner < 50; inner++)
            {
                var outerLocal = outer;
                var innerLocal = inner;
                tasks[inner] = Task.Factory.StartNew(() =>
                    {
                        try
                        {
                            Thread.Sleep(innerLocal);
                            if (innerLocal % 5 == 0)
                            {
                                throw new Exception("Test of " + innerLocal);
                            }
                        }
                        catch (Exception e)
                        {
                            errors.Add(new ErrorInfo
                            {
                                Error = e,
                                Inner = innerLocal,
                                Outer = outerLocal
                            });
                        }
                    });
            }
            Task.WaitAll(tasks);
        }
        Console.WriteLine("Error bag contains {0} errors.", errors.Count);
        Console.ReadLine();
    }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.