-2

I am trying to use ReaderWriterLockSlim to lock some database work in an async method like this:

readerWriterLock.EnterWriteLock();
using (var db = new MyContextDB())
{
    // look something up and alter it
}
readerWriterLock.ExitWriteLock();

This throws an exception as multiple threads try to enter writelock at the same time. Right now I could use a lock object but I thought I would use the ReaderWriterLockSlim so I could try some optimisation with read/write & upgradeable.

The MSDN samples of ReaderWriterLockSlim are a little confusing for me. Is there a simple way to just have the thread wait for the lock to become available? IsWriteLockHeld property says it just tests if the current thread is locked. WaitingWriteCount says it gives the number of threads waiting to enter but how do you make a thread wait to enter? I could not find that in any samples. Am I meant to just loop until it does not throw an exception? That does not seem right.

Guerrilla
  • 13,375
  • 31
  • 109
  • 210
  • 3
    First of all why do you need a lock for local variable? – Sriram Sakthivel May 14 '15 at 14:06
  • *This throws an exception as multiple threads try to enter writelock at the same time.* That should not cause exception. Could you provide complete example, that demonstrate described behavior? – user4003407 May 14 '15 at 14:08
  • I declared lock in class constructor. Should I declare inside each thread? How then do I group locks for multiple databases? – Guerrilla May 14 '15 at 14:09
  • "This throws an exception as multiple threads try to enter writelock at the same time." No it doesn't. Whatever is causing the exception is something else. What exception are you getting? – Jon Hanna May 14 '15 at 14:09
  • Recursive lock exception – Guerrilla May 14 '15 at 14:10
  • `db` is a local variable. You don't even need to lock it. I'm not sure what you're trying to achieve. Why are you locking the access to it, are you accessing any shared state? – Sriram Sakthivel May 14 '15 at 14:11
  • I was getting an issue where the same product would be read and updated at the same time. – Guerrilla May 14 '15 at 14:14
  • *Recursive lock exception* — that mean that you try to take lock, while you already hold one by the same thread. – user4003407 May 14 '15 at 14:15
  • A vote to close because this is not about programming? – Guerrilla May 14 '15 at 14:15
  • @PetSerAl Should I be declaring the RWLS inside the scope of the method? I have one RWLS declared at property that all methods access – Guerrilla May 14 '15 at 14:17
  • No, it should be in a shared scope; if it was scoped to the method it wouldn't actually prevent anything from happening. You need to fix your recursion though, which means either the same thread is trying to get the writer lock or a thread is trying to get the writer lock without first releasing the reader lock. – Jon Hanna May 14 '15 at 14:22
  • Adding locks around some 'database work' is a sure shot way to create undetectable deadlocks. As the deadlock cycle completes in the App space (outside the DB) the deadlock cannot be detected and both the app and the DB embrace in a deadlock waiting each other. Never ever do this. Use solely transactions database locks. – Remus Rusanu May 14 '15 at 14:23
  • @RemusRusanu I have the lock surrounding looking a procuct up and altering its values based on an object in scope then saving it back. What is a "transaction database lock"? – Guerrilla May 14 '15 at 14:29
  • http://en.wikipedia.org/wiki/Optimistic_concurrency_control – Remus Rusanu May 14 '15 at 14:44
  • What backed DB are you dealing with? – Remus Rusanu May 14 '15 at 14:44
  • Database is SQL CE 4.0 – Guerrilla May 14 '15 at 15:10
  • Thanks for link, this is my first app with concurrency so I know I need a better approach, just trying to work my way through it. – Guerrilla May 14 '15 at 15:11

1 Answers1

2

You say in your comment that the exception you are getting is a LockRecursionException.

This means that somewhere either:

  1. A thread is hitting EnterWriteLock while that thread already has a write lock, and the lock was not created with LockRecursionPolicy.SupportsRecursion. Don't create it with LockRecursionPolicy.SupportsRecursion; recursive locks are always a bad idea; fix the fact that you're trying to obtain a lock you already have. (More on why lock recursion is bad is discussed at the answer at https://stackoverflow.com/a/12014173/400547).

  2. A thread is hitting EnterWriteLock when it has a read lock. Release the read lock first.

There are some inclusive/exclusive locks (AKA "reader/writer locks") that allow code to obtain a write lock when they already have a read lock. This though can cause nasty deadlocks as explained in the answer at https://stackoverflow.com/a/8807232/400547 so luckily ReaderWriterLockSlim does not support this. Release your reader lock first, or use an upgradable lock.

Or indeed, since the work covered involves a database, just use a transaction and have the database deal with the concurrency issues.

Community
  • 1
  • 1
Jon Hanna
  • 110,372
  • 10
  • 146
  • 251
  • You were right, I was getting an unrelated exception in part of the code and the catch/finally did not release the lock. I updated that and now it is working fine. Thanks for the help. Really appreciate it – Guerrilla May 14 '15 at 14:25
  • I like to wrap such lock access in disposable objects that `Enter` on construction and `Exit` on disposal, so that `using` prevents such issues happening. That said, since what your wrapping is database work, transactions might be a much better overall solution. – Jon Hanna May 14 '15 at 14:27
  • Incidentally, the bug you got is a great example of why allowing recursion is bad; if your lock supported recursion then instead of getting the exception you'd have had threads waiting on the `Exit` that didn't happen forever, which is much trickier to debug. – Jon Hanna May 14 '15 at 14:29
  • Excellent idea! I will adopt that pattern – Guerrilla May 14 '15 at 14:31