7

I have a dictionary with a fixed collection of keys, which I create at the beginning of the program. Later, I have some threads updating the dictionary with values.

  • No pairs are added or removed once the threads started.
  • Each thread has its own key. meaning, only one thread will access a certain key.
  • the thread might update the value.

The question is, should I lock the dictionary?

UPDATE:

Thanks all for the answers,

I tried to simplify the situation when i asked this question, just to understand the behaviour of the dictionary.

To make myself clear, here is the full version: I have a dictionary with ~3000 entries (fixed keys), and I have more than one thread accessing the key (shared resourse), but I know for a fact that only one thread is accessing a key entry at a time.

so, should I lock the dictionary? and - when you have the full version now, is a dictionary the right choise at all?

Thanks!

Keren
  • 407
  • 2
  • 7
  • 20
  • Yes, you should! Because the dictionary is referenced by the different threads and the whole dictionary is accessed when you update values inside it. – Marcus Oct 15 '13 at 10:00
  • 3
    If each thread basically has it's own part of the dictionary and will never look into the other parts, why is it a dictionary in the first place? Just hold a single variable per thread, local to the thread. – nvoigt Oct 15 '13 at 10:00
  • Depends what type of dictionary this is. Which is it? – Kris Vandermotten Oct 15 '13 at 10:00
  • nvoigt - I tried to simplify the situation. I have more than one thread accessing the key (shared resourse), but I know for a fact that only one thread is accessing a key at a time. – Keren Oct 15 '13 at 10:08
  • Do the concurrent threads, need to share the values they are generarting? Are the values just outputs? What type are the values? If they are reference types, are they immutable? – Jodrell Oct 15 '13 at 10:16
  • If you're not reading/writing the same key concurrent by different threads, you don't need to lock it. (if you do not add/remove keys during usage). But you're on on **thin** ice. I would wrap it in Get/Set methods using a ReaderWriterLock. – Jeroen van Langen Oct 15 '13 at 10:17
  • Will the activity one one thread need to effect the action of another. Will the value changes be used to communicate? – Jodrell Oct 15 '13 at 10:23
  • How do you "know for a fact"? – Jodrell Oct 15 '13 at 10:28

7 Answers7

11

FROM MSDN

A Dictionary can support multiple readers concurrently, as long as the collection is not modified.

To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

For a thread-safe alternative, see ConcurrentDictionary<TKey, TValue>.

Rajesh Subramanian
  • 6,400
  • 5
  • 29
  • 42
3

Let's deal with your question one interpretation at a time.

First interpretation: Given how Dictionary<TKey, TValue> is implemented, with the context I've given, do I need to lock the dictionary?

No, you don't.

Second interpretation: Given how Dictionary<TKey, TValue is documented, with the context I've given, do I need to lock the dictionary?

Yes, you definitely should.

There is no guarantee that the access, which might be OK today, will be OK tomorrow, in a multithreaded world since the type is documented as not threadsafe. This allows the programmers to make certain assumptions about the state and integrity of the type that they would otherwise have to build in guarantees for.

A hotfix or update to .NET, or a whole new version, might change the implementation and make it break and this is your fault for relying on undocumented behavior.

Third interpretation: Given the context I've given, is a dictionary the right choice?

No it isn't. Either switch to a threadsafe type, or simply don't use a dictionary at all. Why not just use a variable per thread instead?

Conclusion: If you intend to use the dictionary, lock the dictionary. If it's OK to switch to something else, do it.

Lasse V. Karlsen
  • 380,855
  • 102
  • 628
  • 825
2

Use a ConcurrentDictionary, don't reinvent the wheel.

Better still, refactor your code to avoid this unecessary contention.


If there is no communication between the threads you could just do something like this:

assuming a function that changes a value.

private static KeyValuePair<TKey, TValue> ValueChanger<TKey, TValue>(
        KeyValuePair<TKey, TValue> initial)
{
    // I don't know what you do so, i'll just return the value.
    return initial;
}

lets say you have some starting data,

var start = Enumerable.Range(1, 3000)
                .Select(i => new KeyValuePair<int, object>(i, new object()));

you could process them all at once like this,

var results = start.AsParallel().Select(ValueChanger);

when, results is evaluated, all 3000 ValueChangers will run concurrently, yielding a IEnumerable<KeyValuePair<int, object>>.

There will be no interaction between the threads, thus no possible concurrency problems.

If you want to turn the results into a Dictionary you could,

var resultsDictionary = results.ToDictionary(p => p.Key, p => p.Value);

This may or may not be useful in your situation but, without more detail its hard to say.

Jodrell
  • 34,946
  • 5
  • 87
  • 124
1

If each thread access only one "value" and if you dont care about others I'll say you dont need a Dictionary at all. You can use ThreadLocal or ThreadStatic variables.

If at all you need a Dictionary you definitely need a lock.

If you're in .Net 4.0 or above I'll strongly suggest you to use ConcurrentDictionary, you don't need to synchronize access when using ConcurrentDictionary because it is already "ThreadSafe".

Sriram Sakthivel
  • 72,067
  • 7
  • 111
  • 189
0

The Diectionary is not thread safe but in your code you do not have to do that; you said one thread update one value so you do not have multi threading problem! I do not have the code so I'm not sure 100%.

Also check this :Making dictionary access thread-safe?

Community
  • 1
  • 1
Bassam Alugili
  • 16,345
  • 7
  • 52
  • 70
0

If you're not adding keys, but simply modifying values, why not completely remove the need for writing directly to the Dictionary by storing complex objects as the value and modifying a value within the complex type. That way, you respect the thread safety constraints of the dictionary.

So:

class ValueWrapper<T>
{
    public T Value{get;set;}
}
//...
var myDic = new Dictionary<KeyType, ValueWrapper<ValueType>>();
//...
myDic[someKey].Value = newValue;

You're now not writing directly to the dictionary but you can modify values.

Don't try to do the same with keys. Necessarily, they should be immutable

Given the constraint "I know for a fact that only one thread is accessing a key entry at a time", I don't think you have any problem.

spender
  • 117,338
  • 33
  • 229
  • 351
  • ok, but you'll need to make `ValueWrapper` thread safe, with possible shortcuts depending on the type of `T`. Essentially, you are reinventing the `ConcurrentDictionary` wheel. – Jodrell Oct 15 '13 at 10:27
  • @Jodrell: yes... ConcurrentDictionary is prob the best solution, but the constraints of the OPs problem lead me to believe that this would be completely safe in this ***specific*** case. ("I know for a fact that only one thread is accessing a key entry at a time") – spender Oct 15 '13 at 10:29
  • If @Keren 's assumption "I know for a fact that only one thread is accessing a key entry at a time" is correct then no synchronization is required. However, without more details, I would be very dubious of that statements veracity. – Jodrell Oct 15 '13 at 10:35
0

Possible modifications of a Dictionary are: add, update and remove.

  1. If the Dictionary is modified or allowed to be modified, you must use a synchronization mechanism of choice to eliminate the potential race condition, in which one thread reads the old dirty value while a second thread is currently replacing the value/updating the key.
    To safe you some work, use the ConcurentDictionary in this scenario.

  2. If the Dictionary is never modified after creation, there won't be any race conditions. A synchronization is therefore not required.
    This is a special scenario in which you can replace the table with a read-only table. To add the important robustness, like guarding against potential bugs by accidentally manipulating the table, you should make the Dictionary immutable (or read-only). To give the developer compiler support, such an immutable implementation must throw an exception on any manipulation attempts.
    To safe you some work, you can use the ReadOnlyDictionary in this scenario. Note that the underlying Dictionary of the ReadOnlyDictionary is still mutable and that its changes are propagated to the ReadOnlyDictionary facade. The ReadOnlyDictionary only helps to ensure that the table is not accidentally modified by its consumers.

This means: Dictionary is never an option in a multithreaded context.
Rather use the ConcurrentDictionary or a synchronization mechanism in general (or use the ReadOnlyDictionary if you can guarantee that the original source collection never changes).

Since you allow and expect manipulations of the table ("[...] the thread might update the value"), you must use a synchronization mechanism of choice or the ConcurrentDictionary.

BionicCode
  • 1
  • 4
  • 28
  • 44