потокобезопасная операция с картой

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. Почему карта должна быть синхронизирована на кластерной клавише - если она не находится на карте как сам монитор?
2. Разве последние пользователи .add ... тоже не могут быть синхронизированы?
3. Кажется, это большой код для добавления отдельного пользователя в поточно-ориентированный способ. Какая реализация будет разумнее?

Answer 1

Вот только некоторые наблюдения:

1. Синхронизация на String - очень плохая идея -> синхронизация на clusterKey и serverKey, вероятно, не будет работатьпуть намечен.
2. Лучше было бы использовать ConcurrentHashMap s и ConcurrentHashSet s.

Хотя без большего контекста на самом деле невозможно ответить на этот вопрос.Кажется, что автор кода хотел безопасно создать только 1 сопоставление на clusterKey и serverKey, чтобы пользователь мог быть добавлен только один раз.

(возможно, лучше) было бы просто synchronize наclusters сама карта, и тогда вы в безопасности, поскольку только один поток может читать и / или записывать на указанную карту.

Другой способ - использовать пользовательские Lock s, возможно, один для чтения, идругой для записи, хотя это может снова привести к несоответствиям, если один поток записывает в Map, в то время как другой читает это точное значение из него.

Answer 2

Код выглядит как не продуманная версия Двойной проверенной идиомы блокировки , которая иногда используется для отложенной инициализации.Прочитайте приведенную ссылку, чтобы узнать, почему это действительно плохая реализация.

Проблема с данным кодом заключается в том, что он периодически дает сбой.Существует состояние гонки, когда несколько потоков пытаются работать с картой, используя один и тот же ключ (или ключи с одинаковым хэш-кодом), что означает, что созданная первая карта может быть заменена второй хэш-картой.

Answer 3

1 -Синхронизация пытается избежать того, чтобы два потока одновременно создавали новую запись в этой карте.Второй должен ждать, чтобы его (servers==null) также не возвращал true.

2 - этот список users, похоже, выходит за рамки, но, похоже, не нуждается в синхронизации.Возможно, программист знает, что дублированных идентификаторов пользователя нет, или, может быть, ему нет дела до сброса одного и того же пользователя снова и снова.

3- ConcurrentHashMap возможно?