tags:

views:

87

answers:

1

I believe this works, I've tested it with multiple concurrent threads (not exhaustively though 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:

http://stackoverflow.com/questions/2318005/can-i-remove-items-from-a-concurrentdictionary-from-within-an-enumeration-loop-of

And this question:

http://stackoverflow.com/questions/2901289/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).
+2  A: 

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.

Jon Skeet
thanks jonyes: dump is meant to be a pointless catchall. i will turn it into a local variable, as a global scope public static is a vulnerabilitythe properties within item: i will lock the individual properties rather than item, as you suggest. there may be a more elegant way to handle this, but these properties will be changed in one thread, read in another, and removed at the item level in a thirdcheers!
circletimessquare