CODEWITHSUNDEEP
c#multithreadinglockingsyncroot
Yarl
Yarl

Reputation: 798

Why is queue lock in this example necessary

I have got the example shown below. I cannot find out why there is lock on queue SyncRoot also while the both coherent algorithms are whole locked using the same object.

It is said that queue lock is necessary.

public class CrudeThreadPool
{
    static readonly int MaxWorkThreads = 4;
    static readonly int WaitTimeout = 2000;

    public delegate void WorkDelegate();

    public CrudeThreadPool() {
        stop = false;
        workLock = new Object();
        workQueue = new Queue();
        threads = new Thread[ MaxWorkThreads ];

        for( int i = 0; i < MaxWorkThreads; ++i ) {
            threads[i] =
                new Thread( new ThreadStart(this.ThreadFunc) );
            threads[i].Start();
        }
    }

    private void ThreadFunc() {
        lock( workLock ) {
            do {
                if( !stop ) {
                    WorkDelegate workItem = null;
                    if( Monitor.Wait(workLock, WaitTimeout) ) {
        
                        lock( workQueue.SyncRoot ) {
                            workItem =
                                (WorkDelegate) workQueue.Dequeue();
                        }
                        workItem();
                    }
                }
            } while( !stop );
        }
    }

    public void SubmitWorkItem( WorkDelegate item ) {
        lock( workLock ) {
            lock( workQueue.SyncRoot ) {
                workQueue.Enqueue( item );
            }

            Monitor.Pulse( workLock );
        }
    }

    public void Shutdown() {
        stop = true;
    }

    private Queue         workQueue;
    private Object        workLock;
    private Thread[]      threads;
    private volatile bool stop;
}

What it the reason for locking on queue SyncRoot, i.e. lock(workQueue.SyncRoot )?

Upvotes: 2

Views: 2477

Answers (1)

Slugart
Slugart

Reputation: 4680

The inner lock is not actually necessary because as long as the Wait is not reached again the lock will be held and will block all producers. Therefore this should work:

private void ThreadFunc() {
   do {
        if( !stop ) {
            WorkDelegate workItem = null;
            lock( workLock ) {
                if( Monitor.Wait(workLock, WaitTimeout) ) {
                    workItem = (WorkDelegate) workQueue.Dequeue();
                }
            }
            if (workItem != null) workItem();
        }
    } while( !stop );
}

public void SubmitWorkItem( WorkDelegate item ) 
{
    lock( workLock ) {
        workQueue.Enqueue( item );

        Monitor.Pulse( workLock );
    }
}

Joseph Albahari's site is an awesome reference for threading scenarios. Though as this is a classic producer/consumer scenario I would recommend you use a BlockingCollection.

Upvotes: 1

Related Questions