Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Lock dictionary within same thread

I have a function that returns an entry on a dictionary, based on the Key (name) and if it doesn't exist, returns a newly created one.

The question I have is with the "double lock" : SomeFunction locks the _dictionary, to check for the existance of the key, then calls a function that also locks the same dictionary, it seems to work but I am not sure if there is a potential problem with this approach.

public Machine SomeFunction(string name) 
{
    lock (_dictionary)
    {
        if (!_dictionary.ContainsKey(name))
                    return CreateMachine(name);
        return _dictionary[name];
    }
}


private Machine CreateMachine(string name)
{
    MachineSetup ms = new Machine(name);
    lock(_dictionary)
    {
        _ictionary.Add(name, ms);
    }
    return vm;
}
like image 709
MexDev Avatar asked Mar 09 '10 19:03

MexDev


People also ask

Do you need to lock concurrent dictionary?

ConcurrentDictionary<TKey,TValue> is designed for multithreaded scenarios. You do not have to use locks in your code to add or remove items from the collection. However, it is always possible for one thread to retrieve a value, and another thread to immediately update the collection by giving the same key a new value.

Is dictionary add thread-safe?

Dictionary is not thread-safe at all, regardless of whether you only add to it or not - there are a few internal structures to it that need to be kept in sync (especially when the internal hashbuckets get resized).

How does Concurrent dictionary work?

ConcurrentDictionary is thread-safe collection class to store key/value pairs. It internally uses locking to provide you a thread-safe class. It provides different methods as compared to Dictionary class. We can use TryAdd, TryUpdate, TryRemove, and TryGetValue to do CRUD operations on ConcurrentDictionary.


2 Answers

That's guaranteed to work - locks are recursive in .NET. Whether it's really a good idea or not is a different matter... how about this instead:

public Machine SomeFunction(string name) 
{ 
    lock (_dictionary)
    {
        Machine result;
        if (!_dictionary.TryGetValue(name, out result))
        {
            result = CreateMachine(name);
            _dictionary[name] = result;
        }
        return result;
    } 
}

// This is now *just* responsible for creating the machine,
// not for maintaining the dictionary. The dictionary manipulation
// is confined to the above method.
private Machine CreateMachine(string name)
{
    return new Machine(name);
}
like image 95
Jon Skeet Avatar answered Oct 27 '22 23:10

Jon Skeet


No problem here, the lock is re-entrant by the same thread. Not all sync objects have thread affinity, Semaphore for example. But Mutex and Monitor (lock) are fine.

like image 32
Hans Passant Avatar answered Oct 27 '22 22:10

Hans Passant