synchronizations of threads

[puzzlecracker]

The following example of rudimentary thread pool is 'bluntly' copied
from "Accelerated C#" book. My questions follow the code:

using System;
using System.Threading;
using System.Collections;
public class CrudeThreadPool
{
static readonly int MAX_WORK_THREADS = 4;
static readonly int WAIT_TIMEOUT = 2000;
public delegate void WorkDelegate();

public CrudeThreadPool() {
stop = 0;
workLock = new Object();
workQueue = new Queue();
threads = new Thread[ MAX_WORK_THREADS ];
for( int i = 0; i < MAX_WORK_THREADS; ++i ) {
threads =new Thread( new
ThreadStart(this.ThreadFunc) );
threads.Start();
}
}
private void ThreadFunc() {
lock( workLock ) {
int shouldStop = 0;
do {
shouldStop = Interlocked.Exchange( ref stop,stop );
if( shouldStop == 0 )
{
WorkDelegate workItem = null;
if( Monitor.Wait(workLock, WAIT_TIMEOUT) ) {
// Process the item on the front of the
queue
lock( workQueue ) {
workItem =(WorkDelegate)
workQueue.Dequeue();
}
workItem();
}
}
} while( shouldStop == 0 );
}
}
public void SubmitWorkItem( WorkDelegate item ) {
lock( workLock ) {
lock( workQueue ) {
workQueue.Enqueue( item );
}
Monitor.Pulse( workLock );
}
}
public void Shutdown() {
Interlocked.Exchange( ref stop, 1 );
}
private Queue workQueue;
private Object workLock;
private Thread[] threads;
private int stop;
}
public class EntryPoint
{
static void WorkFunction() {
Console.WriteLine( "WorkFunction() called on Thread {0}",
Thread.CurrentThread.GetHashCode() );
}
static void Main() {
CrudeThreadPool pool = new CrudeThreadPool();
for( int i = 0; i < 10; ++i ) {
pool.SubmitWorkItem(new
CrudeThreadPool.WorkDelegate(EntryPoint.WorkFunction) );
}
pool.Shutdown();
}
}

SO my question is why do we need need workLock, can we just use
workQueue to synchronize the access to the queue and manage the
execution of threads in the pool?


Thanks

 

[puzzlecracker]

Is that really the code directly from the book, verbatim?  What editionof  
the book are you looking at?

Yes, copied exactly as stated written in the book (page 343)

To answer your immediate question, it's not the lock using workLock that's  
superfluous, it's the lock using workQueue.  In fact, IMHO the lock using  
workQueue shouldn't be used at all.  As has been discussed elsewhere,  
including in Jon Skeet's web article on threading, using a privately  
allocated instance of Object for a lock is preferable to using the object 
that actually needs the synchronized access, because doing so ensures that  
only that particular code is locking on that particular object.

That is a different issue, but I agree that a separate object (aka
lock) instance should be used for synchronization.