none
Accessing global variables through multiple background worker instances RRS feed

  • Question

  • I have written a windows service. The service has a timer. The service is basically a message processing service. When the timer elapses I am disabling the time and I am fetching all messages from the message queue. I have a global variable as threadCount. The total message count is saved in the threadCount parameter. Now I process each message on a different instance of a background worker. Each instance of the background worker is executed asynchronously. Each background worker instance when it gets completed decrements the threadCount variable and when the threadCount becomes zero which indicates that all messages are processed and hence the timer is enabled. Having written this logic. The logic seems to work fine in our local development environments. However I am not very sure if I what I am doing is technically correct and will not pose any problems when I have large number of messages to be processed. Will accessing of same global variable result in any locking errors or unhandled exceptions? Any help / comments on this will be highly appreciated
    I have inserted code block for reference
    private int threadsCount = 0;
            
    protected override void OnStart(string[] args)
    {         Application.SetUnhandledExceptionMode(UnhandledExceptionMode.CatchException);
                AppDomain.CurrentDomain.UnhandledException += new UnhandledExceptionEventHandler(CurrentDomain_UnhandledException);
                timerMessagingService.Interval = Convert.ToDouble(ConfigurationManager.AppSettings["TimerInterval"]);
                timerMessagingService.Enabled = true;
    }
    
    private void timerMessagingService_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
    {
    
       // Code to fetch messages goes here
    
       threadsCount = <MessageCount>;
        if (threadsCount > 0)
        {
            foreach (message in MessageQueue)
           {
                                    BackgroundWorker bgWorker = new    BackgroundWorker();
                            bgWorker.DoWork += new DoWorkEventHandler(bgWorker_DoWork);
                            bgWorker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(bgWorker_RunWorkerCompleted);
     object[] processingInputs = new object[2];
    bgWorker.RunWorkerAsync(processingInputs);
    
            }
         }
    }
    
            void bgWorker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
            {
                --this.threadsCount;
                if (this.threadsCount == 0)
                {
                    timerMessagingService.Enabled = true; 
                }
    }
    
    void bgWorker_DoWork(object sender, DoWorkEventArgs e)
    {
               // Do Some work here
    }
                   
    void CurrentDomain_UnhandledException(object sender, UnhandledExceptionEventArgs e)
    {
        // Do excpetion handling here
    }
    Thursday, November 10, 2011 7:16 AM

Answers

  • Hi Vandana,

       Rocky is correct you should use a lock for threadsCount, everywhere you read or write the value. The general rule here is that if the same object or static field is being accessed by multiple threads, you want to use a lock around any read or write. Being a global doesn't matter, its just whether or not the value is used on multiple threads.

    If you forget the lock often you won't see any error (you definately won't get an exception), but on rare occasion you won't get the result you expect. For example if you set threadCount = 2 and then run 2 threads that each execute --threadCount, when everything is done the result might not be 0. What happened is that

    --threadCount is compiled to machine code such as:

    register a = threadCount;
    a = a-1;
    threadCount = a;
    If two threads execute in parallel on two different processors each of them might simultaneously read '2' from memory, subtract 1 from their local copy, then store the value '1' back to memory. Using a lock ensures that only one thread goes at a time.

     

    Also you may want to test how well your solution performs when you are processing large numbers of messages. Your solution will create a thread per message and large numbers of threads are often undesirable. But first rule of performance, always measure first : ) If you ultimately do need to lower the number of threads, the ThreadPool or the task parallel library can help.

    -Noah

    Monday, November 14, 2011 12:41 PM
    Moderator
  • You call all of the BackgroundWorkers from the same thread and you only access ThreadCount from the Completed event which is on the thread that called the workers.  Since only one completed event can run at a time, TheadCount can only be accessed by one worker at a time.  This is called a SynchronizationContext.  The variable is only accessed on one thread.  Everything on a single thread must occur synchronously. 

    I can tell you from experience that this method of ensuring sychronization if rock solid.  I can't say the same for multi-thread access through a lock.

    Monday, November 14, 2011 6:47 PM

All replies

  • Hello vherkal,

    Welcome to the MSDN forum.

    As I double checked this post, I only find that there may be an exception about the global variable- threadsCount. I would suggest you to lock it when --this.threadsCount.

     

        private object obj = new object();
            void bgWorker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
            {
                lock (obj)
                {
                    --this.threadsCount;
                    if (this.threadsCount == 0)
                    {
                        timerMessagingService.Enabled = true;
                    }
                }
            }
    

    Hope it helps.


    Best Regards,
    Rocky Yue[MSFT]
    MSDN Community Support | Feedback to us
    Get or Request Code Sample from Microsoft
    Please remember to mark the replies as answers if they help and unmark them if they provide no help.

    Friday, November 11, 2011 8:14 AM
    Moderator
  • Hello Rocky,

    Thanks for your inputs on this. I wanted to know if i need to lock all global variables that I use. There are some global objects that I use in bgWorker_DoWork. Should i be using lock for such global objects also?

    Thanks,

    Vandana

     

    Monday, November 14, 2011 4:15 AM
  • No, I think we can put all of them to the lock statement using one lock keyword.

     


    Best Regards,
    Rocky Yue[MSFT]
    MSDN Community Support | Feedback to us
    Monday, November 14, 2011 4:29 AM
    Moderator
  • Hi Vandana,

       Rocky is correct you should use a lock for threadsCount, everywhere you read or write the value. The general rule here is that if the same object or static field is being accessed by multiple threads, you want to use a lock around any read or write. Being a global doesn't matter, its just whether or not the value is used on multiple threads.

    If you forget the lock often you won't see any error (you definately won't get an exception), but on rare occasion you won't get the result you expect. For example if you set threadCount = 2 and then run 2 threads that each execute --threadCount, when everything is done the result might not be 0. What happened is that

    --threadCount is compiled to machine code such as:

    register a = threadCount;
    a = a-1;
    threadCount = a;
    If two threads execute in parallel on two different processors each of them might simultaneously read '2' from memory, subtract 1 from their local copy, then store the value '1' back to memory. Using a lock ensures that only one thread goes at a time.

     

    Also you may want to test how well your solution performs when you are processing large numbers of messages. Your solution will create a thread per message and large numbers of threads are often undesirable. But first rule of performance, always measure first : ) If you ultimately do need to lower the number of threads, the ThreadPool or the task parallel library can help.

    -Noah

    Monday, November 14, 2011 12:41 PM
    Moderator
  • You call all of the BackgroundWorkers from the same thread and you only access ThreadCount from the Completed event which is on the thread that called the workers.  Since only one completed event can run at a time, TheadCount can only be accessed by one worker at a time.  This is called a SynchronizationContext.  The variable is only accessed on one thread.  Everything on a single thread must occur synchronously. 

    I can tell you from experience that this method of ensuring sychronization if rock solid.  I can't say the same for multi-thread access through a lock.

    Monday, November 14, 2011 6:47 PM
  • Rocky's lock is totally unnecessary.  Everything on the thread must run synchronously.
    Monday, November 14, 2011 6:53 PM