Lock is being skipped

Question

I have the following code which does some database work:

[WebMethod]
public void FastBulkAdd(int addmax){
Users[] uploaders = db.Users.Take(addmax).ToArray();

Parallel.ForEach(uploaders, item =>
{
    Account account;
    lock (this)
    {
        account = item.Account;
    }
}

Where every user has 1 account, which is referenced on another table in by DB via a foreign key (I am certain each user has exactly 1 account). I have to lock that bit of code because multi-threaded database connections generate errors. When I run this setting addmax to 1 (allowing 1 thread to execute), it works just fine, but if addmax is greater than 1 and more than one thread executes, account will always be null, which generates an exception later on. It's almost like the lock is being skipped.

Update: I wasn't convinced that account would always be null, so I did the following:

int tries = 0;
while (account == null && tries < 100)
{
    lock (this)
    {
        account = item.Account;
    }
    tries++;
}

And it worked. Not a very neat solution. I'd like to know the cause of the problem so that I can avoid this design hazard in the future.

I can't really add breakpoints inside of the Parallel.ForEach block so I feel like I'm debugging the double slit experiment. — JSideris, Feb 09 '13 at 07:20
Since the database stuff can't be done multi-threaded, why not do a single batch SELECT/INSERT/UPDATE for everything you need from the database, and parallelise only non-DB stuff? The less DB hits you do, the faster your code will be — Patashu, Feb 09 '13 at 07:27
@Patashu Well most of the rest of the code just sets up a giant bulk insert, which I'm finding is very slow when done one at a time. When the application is not multi-threaded, each iteration of the loop takes about 1 second to execute. I'd like to see if I can reduce that through parallelization because a lot of the work really is paralellizable, so I'm experimenting with different values for addmax to find the sweet-spot. — JSideris, Feb 09 '13 at 07:32
item.Account does a DB lookup, right? Could you replace it with a bulk select for all of the uploaders' accounts at once? — Patashu, Feb 09 '13 at 07:34
My guess is that `item.Account` isn't getting populated for some reason related to your ORM (is it entity framework? if you look here, it seems that you should have a different context object for each thread, anyway: http://stackoverflow.com/questions/9415955/c-sharp-working-with-entity-framework-in-a-multi-threaded-server) — phoog, Feb 09 '13 at 07:35
@Patashu Never thought about that. It would probably be much faster. — JSideris, Feb 09 '13 at 07:35
@Patashu Your bulk select is the best answer for this question and speeds my application up on the side. Feel free to post for the checkmark. — JSideris, Feb 09 '13 at 07:46

score 2 · Accepted Answer · answered Feb 09 '13 at 07:49

item.Account does a DB lookup, right? Could you replace it with a bulk select for all of the uploaders' accounts at once? That way you only make one hit to the database to select and one hit to the database to bulk update later, and you don't care about synchronized database access (which costs lots of time with every extra hit, anyway)

score 0 · Answer 2 · edited May 23 '17 at 12:18

0

instead of locking this create a private static object and lock it; you may refer to this thread.

Also, you need to verify that the item.Account is not null. Another problem is that, even you are locking the Account, it seems that you are using it later in this code. Which does not seem right, even you are locking, then it may change later in the section where you are saving it to database as it is not locked. Refer to the following sample;

lock (this)
{
    account = item.Account;
}
DoSomeDatabaseOperation(account); // the account may change here when another thread is also operating.

Also you may debug parallel operations; refer to this msdn page.

edited May 23 '17 at 12:18

Community

1
1

answered Feb 09 '13 at 07:35

daryal

14,643
4
38
54

The account isn't changed later in the code, it is just used to populate data in other tables. Thanks, these were great resources. – JSideris Feb 09 '13 at 07:40
@Bizorke even it is not changing later, if you are using it, it will still be a problem. – daryal Feb 09 '13 at 07:45

score 0 · Answer 3 · answered Feb 09 '13 at 07:59

You can use [MethodImpl(MethodImplOptions.Synchronized)] For example

  [MethodImpl(MethodImplOptions.Synchronized)]
    [WebMethod]
    public void FastBulkAdd(int addmax)
    {
    }

Refer the below link for more details. http://msdn.microsoft.com/en-us/library/system.runtime.compilerservices.methodimploptions.aspx

Lock is being skipped

3 Answers3