ConcurrentDictionary<TKey, HashSet<T>>中的HashSet<T>作为值时，是否线程安全？

Question

如果我有以下代码：

var dictionary = new ConcurrentDictionary<int, HashSet<string>>();

foreach (var user in users)
{
   if (!dictionary.ContainsKey(user.GroupId))
   {
       dictionary.TryAdd(user.GroupId, new HashSet<string>());
   }

   dictionary[user.GroupId].Add(user.Id.ToString());
}

向HashSet中添加元素的行为是否天然线程安全，因为HashSet是并发字典的值属性？

- Marko

Answer 1

不。将一个容器放入线程安全容器中并不能使内部容器变得线程安全。

dictionary[user.GroupId].Add(user.Id.ToString());

在从ConcurrentDictionary检索HashSet后调用add方法可能会导致代码出现奇怪的故障模式，如果同时从两个线程查找此GroupId，则会破坏您的代码。我曾经看到我的一个队友犯了不锁定集合的错误，结果很糟糕。

这是一个可行的解决方案。我个人会做一些不同的事情，但这更接近您的代码。

if (!dictionary.ContainsKey(user.GroupId))
{
    dictionary.TryAdd(user.GroupId, new HashSet<string>());
}
var groups = dictionary[user.GroupId];
lock(groups)
{
    groups.Add(user.Id.ToString());
}

- Joshua

---

@Joshua，为什么你说你会做一些不同的事情？这样锁定哈希集有什么问题吗？有更好的方法吗？ - Marko

@Marko 在工作中，我们有一个更强大的原语来构建它。锁定并不糟糕，只是稍微次优，并且更容易理解。 - Joshua

Answer 2

不，这个集合（词典本身）是线程安全的，但你放进去的内容不一定是安全的。你有几种选择：

1. Use AddOrUpdate as @TheGeneral mentioned:

   dictionary.AddOrUpdate(user.GroupId,  new HashSet<string>(), (k,v) => v.Add(user.Id.ToString());
   

2. Use a concurrent collection, like the ConcurrentBag<T>:

   ConcurrentDictionary<int, ConcurrentBag<string>>
   

每当你构建字典时，就像你的代码一样，最好尽可能少地访问它。可以考虑以下方法：

var dictionary = new ConcurrentDictionary<int, ConcurrentBag<string>>();
var grouppedUsers = users.GroupBy(u => u.GroupId);

foreach (var group in grouppedUsers)
{
    // get the bag from the dictionary or create it if it doesn't exist
    var currentBag = dictionary.GetOrAdd(group.Key, new ConcurrentBag<string>());

    // load it with the users required
    foreach (var user in group)
    {
        if (!currentBag.Contains(user.Id.ToString())
        {
            currentBag.Add(user.Id.ToString());
        }
    }
}

3. 如果你实际上想要一个内置的并发HashSet集合，你需要使用ConcurrentDictionary<int, ConcurrentDictionary<string, string>>，并且关心内部字典中的键或值。

- Camilo Terevinto

---

ConcurrentBag是否像HashSet一样，不允许在列表中存在重复项？ - Marko

@Marko 不，它的行为更像是一个并发列表。 - Camilo Terevinto

@CamiloTerevinto 我看到ConcurrentBag唯一的问题是如何从包中删除项目？没有Remove...看起来我需要使用TryTake？ - Marko

第一个解决方案是不正确的。根据文档，“updateValueFactory委托在锁定之外调用，以避免在锁定下执行未知代码可能导致的问题。”因此，第一个解决方案可能会导致HashSet<string>的损坏或丢失更新。 - Theodor Zoulias

@MichaelRandall 我正在审查标签concurrentdictionary下的问题。顺便说一下，这里是AddOrUpdate方法的源代码链接：https://referencesource.microsoft.com/mscorlib/system/Collections/Concurrent/ConcurrentDictionary.cs.html#490ee025e9652769。不仅updateValueFactory在没有同步的情况下被调用，而且在争用的情况下它还可以被调用多次。 - Theodor Zoulias

第二个解决方案也是不正确的。Contains+Add组合不是原子性的。Contains甚至不是线程安全的，因为“通过ConcurrentBag<T>实现的接口之一访问的成员（包括扩展方法）不能保证是线程安全的”（参考）。此外，Contains是一个O(N)操作，因此不适合替换HashSet。 - Theodor Zoulias