views:

52

answers:

1

Hi, I have a problem regarding multithreading and inserting an item to an Dictionary. The following situation is what I am encoutering when insertingen subjects with duplicate id's:

    private static readonly Timer bufferChecker;
    private static readonly List<SubjectStartRulePair> inBuffer;
    private static readonly IDictionary<Guid, Subject> beingProcessed;
    private static readonly object objLock;

    static InBuffer()
    {
        objLock = new object();
        inBuffer = new List<SubjectStartRulePair>();
        beingProcessed = new Dictionary<Guid, Subject>();
        bufferChecker = new Timer(x => ProcessThreads(), null, 1000, 1000);
    }

    public static void ProcessThreads()
    {
        lock(objLock)
        {
            var bufferedItems = inBuffer.OrderBy(x => x.QueuedTime);
            foreach (var item in bufferedItems)
            {
                if (!beingProcessed.ContainsKey(item.Subject.SubjectId)) //Important check which validates if there is already a Thread running
                {
                    var thread = new Thread(
                        x =>
                        {
                            //Thread #2 is here and runs into duplicate Key
                            beingProcessed.Add(item.Subject.SubjectId, item.Subject); 
                            item.StartRule(item.Subject);
                            beingProcessed.Remove(item.Subject.SubjectId);
                        });

                    thread.Start();
                    inBuffer.Remove(item);
                }
            }
        }
    }

    public static void TryToExecute(Subject subject, IBusinessRule rule)
    {
        lock (objLock)
        {
            if (beingProcessed.ContainsKey(subject.SubjectId)) //Important check which validates if there is already a Thread running
            {
                inBuffer.Add(
                    new SubjectStartRulePair
                        {
                            QueuedTime = DateTime.Now,
                            Subject = subject,
                            StartRule = (x =>
                            {
                                rule.RunChildren(subject);
                                return true;
                            })
                        }
                );
            }
            else
            {
                var thread = new Thread(
                    x =>
                    {
                        beingProcessed.Add(subject.SubjectId, subject);
                        rule.RunChildren(subject);
                        beingProcessed.Remove(subject.SubjectId);
                    });

                thread.Start(); //Thread #1 is here
            }
        }
    }

I have locked both methods, but the lock doesn't seem to work... It seems that two threads both enter the lock on the different methods. Am I missing the point of using lock()? Any idea on how I should implement this correctly? Important sidenote, the ProcessThreads() method is being called every second by a Timer (bufferChecker).

+2  A: 

You're starting a new thread in each method - those new threads won't have (or request) the lock.

So although only one of ProcessThreads or TryToExecute can effectively run at a time, you don't have any control over the bits in the lambda expressions. If those require mutual exclusion as well, you need to put a lock statement in those lambdas.

Jon Skeet
Thanks for the quick awnser Jon! I moved the beingProcessed.Add() outside of the lambda to resolve this issue, which also gives the desired result :]
Bas