0

I have multiple threads running a batch job. When each thread finishes it calls this method of mine:

        private static readonly Object lockVar = new Object();
    public void UserIsDone(int batchId, int userId)
    {
        //Get the batch user
        var batchUser = context.ScheduledUsersBatchUsers.SingleOrDefault(x => x.User.Id == userId && x.Batch.Id == batchId);

        if (batchUser != null)
        {

            lock (lockVar)
            {
                context.ScheduledUsersBatchUsers.Remove(batchUser);
                context.SaveChanges();

                //Try to get the batch with the assumption it has no users left. If we do get the batch back, it means there are no users left.
                var dbBatch = context.ScheduledUsersBatches.SingleOrDefault(x => x.Id == batchId && !x.Users.Any());

                //So this must have been the last user, the batch is empty, so we fetch it and remove it.
                if (dbBatch != null)
                {
                    context.ScheduledUsersBatches.Remove(dbBatch);
                    context.SaveChanges();
                }

            }

        }
}

What this method does is very simple, it looks up the "BatchUser" to remove him from the queue, which it does. That part works swell.

However, after removing the user I want to check if that was the last user in the whole batch. But since this is multithreaded a race condition can happen.

So I put the removing of the batch user within a lock, after I remove the user, I check if the batch has no more batch users.

But here is my problem... even tho I have a lock, and the query to get the "dbBatch" clearly requires it to have no users to return the object... even so, I sometimes get it back with users like so:Getting users when there should be none

When I do get that, I also get the following error on SaveChanges() Store update, insert, or delete statement affected an unexpected number of rows (0). Entities may have been modified or deleted since entities were loaded

However, at other times I get the dbBatch object back correctly with no children, like so: Without Users And when I do, it all works great, no exceptions.

With debugger I can catch the error by setting a breakpoint on the lock statement (see screenshot one). Then all threads get to the lock (while one goes in). Then I always get the error. If I only have a breakpoint inside the if-statement it's more random.

With the lock in place, I don't see how this happens.

Update

I Ninject my context, and this is my ninject code

kernel.Bind<MyContext>()
            .To<MyContext>()
            .InRequestScope()
            .WithConstructorArgument("connectionStringOrName", "MyConnection");

        kernel.Bind<DbContext>().ToMethod(context => kernel.Get<MyContext>()).InRequestScope();

Update 2

I also tried this solution https://msdn.microsoft.com/en-us/data/jj592904.aspx

But strangely I don't get a DbUpdateConcurrencyException but rather I get a DbUpdateException that has an InnerException that is OptimisticConcurrencyException.

But neither DbUpdateException or OptimisticConcurrencyException contains a Entries property so I can't do ex.Entries.Single().Reload();

I'm also adding the exception in text form here

Here in text also, The outer exception of type DbUpdateException: {"An error occurred while saving entities that do not expose foreign key properties for their relationships. The EntityEntries property will return null because a single entity cannot be identified as the source of the exception. Handling of exceptions while saving can be made easier by exposing foreign key properties in your entity types. See the InnerException for details."}

The InnerException of type OptimisticConcurrencyException: {"Store update, insert, or delete statement affected an unexpected number of rows (0). Entities may have been modified or deleted since entities were loaded. See http://go.microsoft.com/fwlink/?LinkId=472540 for information on understanding and handling optimistic concurrency exceptions."}

Rickard Liljeberg
  • 966
  • 1
  • 12
  • 39
  • Where do you initialize your `context` variable? BTW, I'd prefer a text version of the exception. – Gert Arnold May 26 '16 at 18:56
  • I updated both with how I Ninject the context and also added the exception in text form. – Rickard Liljeberg May 26 '16 at 19:24
  • I would love to get a solution to this, but I ran out of time so I simply had to create a second DbContext instance to get it to work. So it works for me now, but not the way I would like it to. – Rickard Liljeberg May 26 '16 at 20:35
  • From what you show I can't see which threads have access to `context`, but I'm pretty sure it's multiple. But a context is not thread safe, so make sure one context belongs to one thread only. – Gert Arnold May 26 '16 at 21:38
  • But with the lock in place, I thought I protected against any thread issues with the context? – Rickard Liljeberg May 27 '16 at 06:11
  • Not if the context is also accessible outside the lock scope. – Gert Arnold May 27 '16 at 06:13
  • I see how that makes sense. This is being run from hangfire and I am a bit unsure about the instance. But I am still a bit surprised because when I add batch with 3 children and then debug it. I see all 3 threads arriving at the lock first before any thread executes what's inside. So no thread should be doing anything with dbcontext in that time span, yet I get the problem. – Rickard Liljeberg May 27 '16 at 06:23
  • Shouldn't you refresh `batchUser` once the lock has been acquired ? – jbl May 27 '16 at 11:56
  • It should not be needed as this function is only called once per batchUser so no other thread will mess with it – Rickard Liljeberg May 27 '16 at 20:32

0 Answers0