Using lock statement with ThreadPool in C#

问题:

I have a multi-threaded program (C#) where I have to share global static variables between threads that may take some time to execute (sending data request to another system using WCF). The problem is that using the lock statement does not seem to guarantee mutual exclusion when it's declared outside of the ThreadPool.

static void Main(string[] args)
{
    public static int globalVar = 0;
    public object locker;

    System.Timers.Timer timer1 = new System.Timers.Timer(1000);
    timer1.Elapsed += new ElapsedEventHandler(onTimer1ElapsedEvent);
    timer1.Interval = 1000;
    timer1.Enabled = true;

    System.Timers.Timer timer2 = new System.Timers.Timer(500);
    timer2.Elapsed += new ElapsedEventHandler(onTimer2ElapsedEvent);
    timer2.Interval = 500;
    timer2.Enabled = true;
}

public void onTimer1ElapsedEvent(object source, ElapsedEventArgs e)
{
    lock (locker) {
        ThreadPool.QueueUserWorkItem(new WaitCallback(state => 
        {
            globalVar = 1;
            Console.WriteLine("Timer1 var = {0}", globalVar);
        }));
    } 
}
public void onTimer2ElapsedEvent(object source, ElapsedEventArgs e)
{
    lock (locker) {
        ThreadPool.QueueUserWorkItem(new WaitCallback(state => 
        {
            globalVar = 2;
            Thread.Sleep(2000);  // simulates a WCF request that may take time
            Console.WriteLine("Timer2 var = {0}", globalVar);
        }));
    } 
}

So the lock does not work and the program can prints: Timer2 var = 1

Putting the the lock statement inside the ThreadPool seems to resolve the problem.

public void onTimer1ElapsedEvent(object source, ElapsedEventArgs e)
{
    ThreadPool.QueueUserWorkItem(new WaitCallback(state =>
    {
        lock (locker) {           
            globalVar = 1;
            Console.WriteLine("Timer1 var = {0}", globalVar);
        } 
    }));    
}
public void onTimer2ElapsedEvent(object source, ElapsedEventArgs e)
{
    ThreadPool.QueueUserWorkItem(new WaitCallback(state =>
    {
        lock (locker) {           
            globalVar = 2;
            Thread.Sleep(2000);  // simulates a WCF request that may take time
            Console.WriteLine("Timer2 var = {0}", globalVar);
        } 
    }));    
}

However, I don't uderstand the difference between the two approaches and why it does not produce the same behaviour.

Also, the 2nd approach resolves the mutual exclusion problem but the timer1 thread will always have to wait for the timer2 to finish his lock statement (which takes time), so the multi-threading concept does not work anymore in my program. I want to know what's the best solution of having multi threading doing their job in parallel with using shared variables ?

回答1:

You don't need a lock to update variables like that. For example, you can replace this:

lock (locker)
{
    globalVar = 1;
    Console.WriteLine("Timer1 var = {0}", globalVar);
}

with:

int val = 1;
globalVar = val;
Console.WriteLine("Timer1 var = {0}", val);

Writes to primitive types are guaranteed to be atomic, so there's no need to lock here.

Now, if you want to increment a value, you can write:

int val = Interlocked.Increment(ref globalVar);

You can also add:

int val = Interlocked.Add(ref globalVar, 100);

Again, these don't need locks.

Check out the Interlocked class.

回答2:

In your first scenario, all you're locking on is the additional of a new WaitCallback onto the ThreadPool. Think of the ThreadPool as a line: all you've done is lock on putting someone else in line (which ironically is actually double work since the ThreadPool itself locks on an internal queue that it maintains). The code that the ThreadPool executes afterwards is on a different thread, happens at a different time, and has nothing to do with that lock anymore.

In your second scenario, the lock is actually in the code that the ThreadPool thread is executing, which is why you're seeing the locking semantics that are expected.

In general, however, I'd recommend against locking in a ThreadPool thread if you can avoid it. The ThreadPool should be used (ideally) for quick running tasks. It depends on the nature and use of the shared state, and what you're trying to accomplish, but in general I'd opt for using Tasks and/or PLINQ when possible.

回答3:

The shorter and more sensible solution is not to use (yet another) extra thread to execute the Timer. System.Timers.Timer already allocates a pool thread.

public void onTimer1ElapsedEvent(object source, ElapsedEventArgs e)
{
    lock (locker) {
            globalVar = 1;
            Console.WriteLine("Timer1 var = {0}", globalVar);

    } 
}
public void onTimer2ElapsedEvent(object source, ElapsedEventArgs e)
{
    lock (locker) {
            globalVar = 2;
            Thread.Sleep(2000);  // simulates a WCF request that may take time
            Console.WriteLine("Timer2 var = {0}", globalVar);
    } 
}

Your confusion comes from formulations like "Putting the the lock statement inside the ThreadPool".

You put lock statements inside methods to control the threads they are run on.