C# Lock not allowing critical code to run correctly

I spin off a new thread that reads data from a database. Every X records a flag is signaled where the main thread will then process the records that have been retrieved keeping some and discarding others. When the flag is signaled I use a lock to allow the reader thread to wait until the processor thread to finish processing the records. However, it appears the lock is not doing this because as I iterate through the r开发者_Go百科ecords, more keep getting added (indicating the reader thread is still reading). This causes the collection to be modified resulting in an InvalidOperationExecption.

Maybe I have misunderstood what the purpose of "lock" is or I am not using it correctly. Here is pseudo of what I have:

readonly object locker = new object();
Dictionary screened = new Dictionary;

Search(){
    Thread reader = new Thread( () => Read("search terms") );
    reader.Start();

    while( found < desiredAmount ){
        if(SIGNAL){
            lock(locker){
                ProcessRecords();
            }
        }
     }
}

Read(){
    Connect to DB
    while(reader.Read()){
        screened.add(record);
    }
}

ProcessRecords(){
    foreach(var x in screened){
        //process record 
    }
}

I hope the pseudo was good enough, from my understanding Read() should not execute while in the lock block. Please help me understand lock a little better.

PS Yes I have read MSDNs articles on locks and still do quite grasp how to use lock in more complex situations.

---

You'll need to put a lock around the while loop as well. The lock would work if you have two or more threads contending for the same lock, in your above sample, you have no contention, because no other thread requests a lock apart from the first.

Read(){
    Connect to DB
        while(reader.Read()){
            lock(locker)
                screened.add(record);
        }

}

A better way would be to put the lock inside ProcessRecords() as well.

---

screened.Add() is not protected, AFAICT

Try adding the lock like so:

while(reader.Read()){
    lock(locker)
    {
        screened.add(record);
    }
}

---

I am not sure I understand your code... but IF you use a Dictionary to store + read the data then I highly recommend using ConcurrentDictionary - it is thread-safe and really fast (since most operations are implemented lock-free)...

For information see:

• http://www.dotnetperls.com/concurrentdictionary
• http://geekswithblogs.net/BlackRabbitCoder/archive/2011/02/17/c.net-little-wonders-the-concurrentdictionary.aspx

---

You need to lock on the same object in both threads if you want them to be mutually exclusive.

So, at a high level, you want to repeat the following steps until all records are processed:

• acquire lock in your reader thread
• read some records
• acquire lock in your main thread
• process the records

---

As it stands with one collection in use, if you keep any accepted entries in it, the processing will re-process them repeatedly.

A better approach would be a producer/consumer queue, possibly using framework 4.0's BlockingCollection<T>.

The general idea of producer/consumer queue is that the database reader would put items into the queue and the processing would remove them from the queue, and if kept, put them in a different collection.

---

Normally, a lock is used to protect data from being accessed by multiple threads at the same time.

Each part of the code that needs to access the data must first acquire the lock.

A better way to do what you are trying to do would be to pass messages between your threads.

Let's say both of your threads (reader and searcher) each have acces to the same System.Collections.Concurrent.ConcurentQueue. Once your reader has the required number of rows, it puts a collection of objects or whatever on the queue.

Your searcher thread tries to get items out of the queue using the TryDeque function. When it can't get an item, it sleeps, when it can get an item, it processes it.

Putting a larger group of rows on the queue will probably yield better performances because your threads won't spend a lot of time trying to acquire the lock.