3

I have a dictionary that is shared among number of threads. Every thread reads specific value from the dictionary according to a given key, but - if the key does not exist in the dictionary the thread need to add it to the dictionary.
To solve the sync problem, I though to use ReaderWriterLockSlim class which basically gives me readers-writers lock synchronization (meaning readers can run in parallel but only one writer at a time...) but adds upgrade option for a reader. Using the upgrade option I can test whether a given key is already in the dictionary and if not - upgrade the lock and write it, promising only one addition for every key.

My problem is that I cant create two upgradeable locks at a time - meaning this solution is no good... :(

Can somebody please explain to me why Microsoft chose to implement the upgradable lock this way (that I cant have more than one upgradable lock at a time...), and give me any idea how can I implement the upgradable lock by myself \ give me another idea to sync my shared dictionary?

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
ET.
  • 1,899
  • 2
  • 18
  • 28
  • I'm not sure asking someone to explain why Microsoft chose a method is a good question. However, here's an implementation of read/write locked objects. https://github.com/legomaster/Magnum/tree/master/src/Magnum/Threading – Travis Sep 26 '11 at 20:07
  • It wouldn't be *slim* otherwise. Just use a regular ReaderWriterLock. – Hans Passant Sep 26 '11 at 20:21
  • 1
    I can't speak to Microsoft's official reason, but a good reason for disallowing multiple threads holding an upgradeable lock simultaneously is that you have a massive chance of deadlocking. For example, two threads both hold upgradeable locks. Both of them try to upgrade to write locks at the same time (and obtaining a write lock requires no other thread holding write/read/upgradable locks to that resource). They deadlock waiting for each other to let go of the read lock they're both already in. – Chris Hannon Sep 26 '11 at 20:21
  • If you could enter the Upgradeable lock concurrently you could have a race condition on a writer, which is the scenario Upgradeable mode is designed to prevent. See: http://stackoverflow.com/a/26578074/1945631 – Andy Brown Oct 26 '14 at 21:21

4 Answers4

8

If you are using .NET 4.0 why not use the ConcurrentDictionary

hollystyles
  • 4,979
  • 2
  • 36
  • 38
4

I have no idea why ReaderWriterLockSlim was implemented in that way. I suspect there are good reasons.

Why not just use ConcurrentDictionary? Then you don't have to worry about explicit locking.

That said, I don't see where having multiple upgradable reader locks would help you. Consider the following scenario:

Thread1 enters the lock in upgradeable mode
Thread2 enters the lock in upgradeable mode
Thread1 searches for "xyzzy" and doesn't find it
Thread2 searches for "xyzzy" and doesn't find it
Thread2 upgrades to a write lock
Thread1 waits to upgrade to a write lock
Thread2 updates and releases the lock
Thread1 acquires the write lock and overwrites what Thread2 had written

In order to prevent Thread1 from overwriting what Thread2 did, you'd have to write this logic:

Enter upgradable read lock
if (!dict.TryGetValue(...))
{
    Enter write lock
    if (!dict.TryGetValue(...))  // extra check required!
    {
    }
}

Which is exactly what you have to do when there are no upgradeable locks.

Jim Mischel
  • 131,090
  • 20
  • 188
  • 351
  • A downvote is usually accompanied by a reason for the downvote. – Jim Mischel Sep 26 '11 at 20:31
  • Jim: Thanks for the answer, sounds good to me - dont know who did the down vote, sure wasnt me :) I still dont mark it as an answer though, because I would like to know more about the ConcurrentDictionary and do some more reading. Thanks again! – ET. Sep 26 '11 at 20:43
1

UpgradeableReadLock exists so you do not have to release your read lock prior to acquiring a write lock. It lets you do this:

   locker.EnterUpgradeableReadLock();
   ...
   locker.EnterWriteLock(); 
   ...

instead of

   locker.EnterReadLock();
   ...
   locker.ExitReadLock();
   locker.EnterWriteLock();
   ...

As it is a ReaderWriter lock, you may still only have one writer at any given time, which an upgradeable read lock enforces. This means these two aren't equivalent; the first lets other callers sneak in, but allows concurrency in the read portion.

If you have cases where you can use ReadLocks for most reads, and UpgradeableRead for data updates/inserts, this is the intent. If all of your data accesses are potential writers, then this probably doesn't work as well and you can use simple lock(object) around your writes to enforce the exclusive access when adding/updating.

Joe
  • 41,484
  • 20
  • 104
  • 125
-2

It doesn't sound like you are perhaps using the optimal solution to this problem.

Example:

  protected static object _lockObj = new object();

  if(_dictionary.ContainsKey(key))
  {
      return _dictionary[key];
  }
  else
  {
      lock(_lockObj)
      {
           if(_dictionary.ContainsKey(key))
              return _dictionary[key];

           _dictionary.Add(key, "someValue");
           return "someValue";
      }
  }

If you're using .NET 4, try using the ConcurrentDictionary class that others have mentioned.

Tejs
  • 40,736
  • 10
  • 68
  • 86
  • Tejs: thank you for the answer. Your answer is similar to Jim's, and I find nothing wrong with it. I really dont know who downvoted it and why... – ET. Sep 26 '11 at 20:46
  • Note that there is a race condition in the above code, between the first `ContainsKey` and `Add`. It is possible that one thread is doing the first check while another is in the lock performing the `Add`. – Bojan Resnik Sep 27 '11 at 04:09
  • @Tejs A double checked lock is not feasible when reading / writing from a dictionary, your code snippet is flawed. The call to `ContainsKey` is outside of a `lock`, if another thread currently has the lock and is in the process of adding an item and the dictionary is being resized internally the call to `ContainsKey` could cause all sorts of problems. FYI downvote wasn't mine, but figured I'd explain at least a rationale behind it since the downvoter wasn't courteous enough to do so. – Rich O'Kelly Mar 04 '13 at 14:33