CODEWITHSUNDEEP
c#multithreadinglocking.net-4.0concurrentdictionary
circletimessquare
circletimessquare

Reputation: 51

ConcurrentDictionary: TryRemove within a lock?

I believe this works, I've tested it with multiple concurrent threads (though not exhaustively for race conditions and deadlocks):

public static System.Collections.Concurrent.ConcurrentDictionary<string, item> dict =
        new System.Collections.Concurrent.ConcurrentDictionary<string, item>();
public static item dump;

...

foreach (System.Collections.Generic.KeyValuePair<string, item> x in dict)
{
    lock (x.Value)
    {
        if (x.Value.IsCompleted)
        {
            dict.TryRemove(x.Key, out dump);
        }
    }
}

This question is sort of a continuation of this question:

Can I remove items from a ConcurrentDictionary from within an enumeration loop of that dictionary?

And this question:

Updating fields of values in a ConcurrentDictionary

In that I'm doing two "dicey" maneuvers:

  1. Removing values from a ConcurrentDictionary while at the same time enumerating through it (which seems to be ok).
  2. Locking the Value portion of a ConcurrentDictionary. Necessary because manipulating fields of the value is not thread safe, only manipulating the values themselves of the ConcurrentDictionary is thread safe (the code above is a snippet of a larger code block in which fields of values are actually manipulated).

Upvotes: 4

Views: 4393

Answers (2)

J. Lennon
J. Lennon

Reputation: 3361

One great alternative that I found recently it is about remove just if is the key/value match, which means that you can guaranteed the removal of one object (preferable a value type, not a reference type) that there was no change. Example:

 ConcurrentDictionary<string, int> cc = new ConcurrentDictionary<string, int>();

        cc.GetOrAdd("1", 1);

Now suppose that the two lines below are running in a concurrent / multithreaded scenario.

        cc.AddOrUpdate("1", 2, (a,b) => b + 1);

         var removeSuccess = ((ICollection<KeyValuePair<string, int>>)cc).Remove(
    new KeyValuePair<string, int>("1", 1));

If the value doesn't change you will remove "1" with successfully, otherwise will fail because you already have a new value for the same key.

For more information check below: http://thargy.com/2012/09/the-hidden-secret-of-the-concurrent-dictionary/ or http://blogs.msdn.com/b/pfxteam/archive/2011/04/02/10149222.aspx

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1500825

Removing values from a concurrent dictionary while iterating over it is fine. It may have some performance implications (I'm not sure) but it should work.

Note that you're not locking on something internal to the ConcurrentDictionary - you're locking on the monitor associated with an item object. I personally wouldn't want to do that: either these items should be thread-safe (making it okay to manipulate them anyway) or (preferrably) immutable so you can observe them from any thread without locking. Or you could just make the individual property you're checking thread-safe, of course. Document whatever you do!

Finally, your use of out dump seems slightly dubious. Is the point just to have something to give TryRemove? If so, I'd use a local variable instead of a static one.

Upvotes: 4

Related Questions