线程安全映射操作

Question

我遇到了以下代码，并注意到一些不一致之处 - 对于多线程安全代码。

    Map<String,Map<String,Set<String>> clusters = new HashMap<.........>;
    Map<String,Set<String>> servers = clusters.get(clusterkey);
    if(servers==null){
      synchronized(clusterkey){
       servers = clusters.get(clusterkey);
       if(servers==null){....initialize new hashmap and put...}
      }
    }
    Set<String> users=servers.get(serverkey);
    if(users==null){
      synchronized(serverkey){
       users=servers.get(serverkey);
       if(users==null){ ... initialize new hashset and put...}
      }
    }
    users.add(userid);

为什么要在 clusterkey 上同步地图 - 它不应该作为监视器本身在地图上吗？
最后一个users.add...不应该同步吗？
在线程安全的情况下添加单个用户似乎有很多代码manner.What会是更聪明的实现吗？

Answer 1

1 - 同步试图避免两个线程同时在该映射中创建一个新条目。第二个必须等待，这样他的 (servers==null) 也不会 return true.

2 - users 列表似乎超出范围，但似乎不需要同步。也许程序员知道没有重复的用户 ID，或者他不关心一次又一次地重置同一个用户。

3- ConcurrentHashMap 也许吧？

Answer 2

这里只是一些观察：

Synchronizing on a String is a very bad idea -> 在 clusterKey 和 serverKey 上同步可能无法按预期方式工作。
最好使用 ConcurrentHashMaps 和 ConcurrentHashSets。

虽然没有更多的上下文，但实际上不可能回答这个问题。代码作者似乎想安全地为每个 clusterKey 和 serverKey 创建 1 个映射，这样用户就可以添加一次。

一种（可能更好）的方法是在 clusters 地图本身上 synchronize 然后你是安全的，因为只有一个线程可以读取 and/or 写入所述地图.

另一种方法是使用自定义 Locks，可能一个用于读取，另一个用于写入，尽管如果一个线程正在写入 Map 而这可能会再次导致不一致另一个正在从中读取确切的值。

Answer 3

该代码看起来像是 Double checked locking idiom 的未经深思熟虑的版本，有时用于延迟初始化。阅读提供的 link 了解为什么这是一个非常糟糕的实现。

给定代码的问题是它会间歇性地失败。当有多个线程尝试使用相同的键（或具有相同哈希码的键）在映射上工作时，就会出现竞争条件，这意味着首先创建的映射可能会被第二个哈希映射替换。

线程安全映射操作

thread safe map operation

java

hashmap

synchronized