java - 线程安全映射操作

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);

1. 为什么 map 要在 clusterkey 上同步 - 它不应该作为监视器本身在 map 上吗？
2. 最后的 users.add... 是否也应该同步？
3. 以线程安全的方式添加单个用户的代码似乎很多。更聪明的实现方式是什么？

Answer 1

这里只是一些观察:

1. Synchronizing on a String is a very bad idea -> 在 clusterKey 和 serverKey 上同步可能无法按预期方式工作。
2. 更好的方法是使用 ConcurrentHashMap 和 ConcurrentHashSet。

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

一种(可能更好的)方法是在 clusters 映射本身上进行同步，然后你就安全了，因为只有一个线程可以读取和/或写入说 map 。

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