Синхронизированные и локальные копии переменных

Question

Я смотрю на устаревший код, который имеет следующую идиому:

Map<String, Boolean> myMap = someGlobalInstance.getMap();
synchronized (myMap) {
    item = myMap.get(myKey);
}

Предупреждение, которое я получаю при проверке кода Intelli-J:

Synchronization on local variable 'myMap'

Является ли это подходящей синхронизацией и почему?

Map<String, Boolean> myMap = someGlobalInstance.getMap();
synchronized (someGlobalInstance.getMap()) {
    item = myMap.get(myKey);
}

Answer 1

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

Если объект, возвращенный someGlobalInstance.getMap() всегда один и тот же, то синхронизированный блок фактически использует этот монитор квазиглобальных объектов, и код выдает ожидаемый результат.

Я также согласен с предложением использовать синхронизированную обертку, если вам нужно только синхронизировать get()/put() вызовы и не имеют больших синхронизированных блоков.Но убедитесь, что Карта только доступ через оболочку, иначе у вас будет еще один шанс на ошибку.

Также обратите внимание, что если someGlobalInstance.getMap() делает нет возвращайте один и тот же объект все время, тогда даже ваш второй пример кода не будет работать правильно, он может быть даже хуже, чем ваш исходный код, поскольку вы можете синхронизировать объект, отличный от того, который вы вызываете get() на.

Answer 2

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

Answer 3

Я думаю, что было бы лучше использовать синхронизированная оболочка для вашей карты

Answer 4

Алекс прав в том, что добавляет синхронизированную оболочку, вызывая Collections.synchronizedMap(Map) здесь типичный подход.Однако, если вы воспользуетесь этим подходом, все равно могут возникнуть ситуации, когда вам потребуется синхронизировать Mapзамок;напримерпри переборе карты.

Map<String, String> syncMap = Collections.synchronizedMap(new HashMap<String, String>());

// Synchronized on map to prevent ConcurrentModificationException whilst iterating.
synchronized (syncMap) {
  for (Map.Entry<String, String> entry : syncMap.entrySet()) {
    // Do work
  }
}

В вашем примере предупреждение IDEA можно игнорировать, поскольку очевидно, что ваша локальная переменная: map извлекается откуда-то еще (someGlobalInstance), а не создаваться внутри метода, и поэтому потенциально к нему можно получить доступ из других потоков.