C# Quartz Race Condition

I am automating some tasks on my website, but I'm currently stuck.

    public void Execute(JobExecutionContext context)
    {
            var linqFindAccount = from Account in MainAccounts
                                  where Account.Done == false
                                  select Account;

            foreach (var acc in linqFindAccount)
            {
                acc.Done = true;
                // stuff
            }
     }

The issue is that when I start multiple threads the first threads get assigned to the same first account because they set the Done value to true at the same time. How am I supposed to avoid this?

EDIT:

    private  object locker = new object();

    public void Execute(JobExecutionContext context)
    {
        lock (locker)
        {
            var linqFindAccount = from Account in MainAccounts
                                  where Account.Done == false
                                  select Account;

            foreach (var acc in linqFindAccount)
            {
                Console.WriteLine(context.JobDetail.Name + " assigned to " + acc.Mail);
                acc.Done = true;
                // stuff
            }

         }
     }


 Instance [ 2 ] assigned to firstmail@hotmail.com
 Instance [ 1 ] as开发者_JS百科signed to firstmail@hotmail.com

First two threads got assigned to the first account, even though the list contains 30 accounts.

Thanks.

Use

private static readonly object locker = new object();

instead of

private  object locker = new object();

Your problem is that the deferred execution happens when you start the foreach loop. So the result is cached and not reevaluated every loop. So every thread will work with it's own list of the items. So when an Account is set to done, the other list still remain with the object in it.

A Queue is more suitable in this case. Just put the items in a shared Queue and let the loops take items of the queue and let them finish when the Queue is empty.

Few problems with your code:

1) Assuming you use stateless Quartz jobs, your lock does not do any good. Quartz creates new job instance every time it fires a trigger. That is why you see the same account processed twice. It would only work if you use stateful job (IStatefulJob). Or make lock static, but read on.

2) Even if 1) is fixed, it would defeat the purpose of having multiple threads because they will all wait for each other on the same lock. You might as well have one thread doing this.

I don't know enough about requirements especially what's going on in // stuff. It maybe that you don't need this code to run on multiple threads and sequential execution will do just fine. I assume this is not the case and you want to run it multiple threads. The easiest way is to have only one Quartz job. In this job, load Accounts in chunks, say 100 jobs in every chunk. This will give you 5 chunks if you have 500 accounts. Offload every chunk processing to thread pool. It will take care of using optimal number of threads. This would be a poor man's Producer Consumer Queue.

public void Execute(JobExecutionContext context) {

    var linqFindAccount = from Account in MainAccounts
                            where Account.Done == false
                            select Account;
    IList<IList<Account>> chunks = linqFindAccount.SplitIntoChunks(/* TODO */);

    foreach (IList<Account> chunk in chunks) {
        ThreadPool.QueueUserWorkItem(DoStuff, chunk);
    }
}

private static void DoStuff(Object parameter) {
    IList<Account> chunk = (IList<Account>) parameter;

    foreach (Account account in chunk) {
        // stuff
    }
}

As usual, with multiple threads you have to be very careful with accessing mutable shared state. You will have to make sure that everything you do in 'DoStuff' method will not cause undesired side effects. You may find this and this useful.

 foreach (var acc in linqFindAccount)
        {
            string mailComponent = acc.Mail;
            Console.WriteLine(context.JobDetail.Name + " assigned to " + mailComponent);
            acc.Done = true;
            // stuff
        }

Try above.