Blok zsynchronizowany z językiem Java a kolekcje.synchronizedMap

Czy poniższy kod jest skonfigurowany, aby poprawnie zsynchronizować połączenia synchronizedMap?

public class MyClass {
  private static Map<String, List<String>> synchronizedMap = Collections.synchronizedMap(new HashMap<String, List<String>>());

  public void doWork(String key) {
    List<String> values = null;
    while ((values = synchronizedMap.remove(key)) != null) {
      //do something with values
    }
  }

  public static void addToMap(String key, String value) {
    synchronized (synchronizedMap) {
      if (synchronizedMap.containsKey(key)) {
        synchronizedMap.get(key).add(value);
      }
      else {
        List<String> valuesList = new ArrayList<String>();
        valuesList.add(value);
        synchronizedMap.put(key, valuesList);
      }
    }
  }
}

Z mojego zrozumienia potrzebuję zsynchronizowanego bloku, addToMap()aby uniemożliwić wywołanie innego wątku remove()lub containsKey()zanim przejdę przez wywołanie, put()ale nie potrzebuję zsynchronizowanego bloku, doWork()ponieważ inny wątek nie może wejść do zsynchronizowanego bloku addToMap()przed remove()powrotem, ponieważ pierwotnie utworzyłem mapę z Collections.synchronizedMap(). Czy to jest poprawne? Czy jest lepszy sposób, aby to zrobić?

Collections.synchronizedMap() gwarantuje, że każda niepodzielna operacja, którą chcesz uruchomić na mapie, zostanie zsynchronizowana.

Jednak uruchamianie dwóch (lub więcej) operacji na mapie musi być zsynchronizowane w jednym bloku. Więc tak - synchronizujesz się poprawnie.

Jeśli używasz JDK 6, możesz chcieć wypróbować ConcurrentHashMap

Zwróć uwagę na metodę putIfAbsent w tej klasie.

Istnieje możliwość wystąpienia subtelnego błędu w kodzie.

[ AKTUALIZACJA: Ponieważ używa map.remove (), ten opis nie jest całkowicie poprawny. Brakowało mi tego faktu za pierwszym razem. :( Dziękuję autorowi pytania za wskazanie tego. Resztę zostawiam bez zmian, ale zmieniłem główne stwierdzenie, aby powiedzieć, że jest potencjalnie błąd.]

W doWork () uzyskujesz wartość List z mapy w sposób bezpieczny dla wątków. Później jednak uzyskujesz dostęp do tej listy w niebezpiecznej sprawie. Na przykład, jeden wątek może używać listy w doWork (), podczas gdy inny wątek wywołuje synchronizedMap.get (klucz) .add (wartość) w addToMap () . Te dwa rodzaje dostępu nie są zsynchronizowane. Ogólna zasada jest taka, że ​​gwarancje bezpieczeństwa wątków kolekcji nie obejmują kluczy ani wartości, które przechowują.

Możesz to naprawić, wstawiając zsynchronizowaną listę do mapy, np

List<String> valuesList = new ArrayList<String>();
valuesList.add(value);
synchronizedMap.put(key, Collections.synchronizedList(valuesList)); // sync'd list

Alternatywnie możesz zsynchronizować się na mapie podczas korzystania z listy w doWork () :

  public void doWork(String key) {
    List<String> values = null;
    while ((values = synchronizedMap.remove(key)) != null) {
      synchronized (synchronizedMap) {
          //do something with values
      }
    }
  }

Ostatnia opcja nieco ograniczy współbieżność, ale jest nieco bardziej przejrzysta IMO.

Krótka uwaga na temat ConcurrentHashMap. Jest to bardzo przydatna klasa, ale nie zawsze jest odpowiednim zamiennikiem dla zsynchronizowanych HashMaps. Cytując z Javadocs,

Ta klasa jest w pełni interoperacyjna z Hashtable w programach, które polegają na bezpieczeństwie wątków, ale nie na szczegółach synchronizacji .

Innymi słowy, putIfAbsent () świetnie nadaje się do wstawiania atomowego, ale nie gwarantuje, że inne części mapy nie zmienią się podczas tego wywołania; gwarantuje tylko atomowość. W swoim przykładowym programie polegasz na szczegółach synchronizacji (zsynchronizowanej) mapy HashMap dla rzeczy innych niż put ().

Ostatnia rzecz. :) Ten wspaniały cytat z Java Concurrency in Practice zawsze pomaga mi w projektowaniu debugowania programów wielowątkowych.

Dla każdej zmiennej stanu, do której można uzyskać dostęp przez więcej niż jeden wątek, wszystkie dostępy do tej zmiennej muszą być wykonywane z tą samą blokadą.

Tak, synchronizujesz poprawnie. Wyjaśnię to bardziej szczegółowo. Musisz zsynchronizować dwa lub więcej wywołań metod w obiekcie synchronizedMap tylko w przypadku, gdy musisz polegać na wynikach poprzedniego wywołania metody w kolejnym wywołaniu metody w sekwencji wywołań metod w obiekcie synchronizedMap. Spójrzmy na ten kod:

synchronized (synchronizedMap) {
    if (synchronizedMap.containsKey(key)) {
        synchronizedMap.get(key).add(value);
    }
    else {
        List<String> valuesList = new ArrayList<String>();
        valuesList.add(value);
        synchronizedMap.put(key, valuesList);
    }
}

W tym kodzie

synchronizedMap.get(key).add(value);

i

synchronizedMap.put(key, valuesList);

wywołania metod opierają się na wyniku poprzedniego

synchronizedMap.containsKey(key)

wywołanie metody.

Jeśli sekwencja wywołań metod nie została zsynchronizowana, wynik może być nieprawidłowy. Na przykład thread 1wykonuje metodę addToMap()i thread 2wykonuje metodę doWork() Sekwencja wywołań metod na synchronizedMapobiekcie może wyglądać następująco: Thread 1wykonał metodę

synchronizedMap.containsKey(key)

a wynik to „ true”. Po tym, jak system operacyjny przełączył kontrolę wykonywania na thread 2i wykonał

synchronizedMap.remove(key)

Po tym sterowanie wykonaniem zostało przełączone z powrotem na thread 1i na przykład zostało wykonane

synchronizedMap.get(key).add(value);

wierząc, że synchronizedMapobiekt zawiera keyi NullPointerExceptionzostanie wyrzucony, ponieważ synchronizedMap.get(key) powróci null. Jeśli sekwencja wywołań metod na synchronizedMapobiekcie nie jest zależna od wzajemnych wyników, nie ma potrzeby synchronizowania sekwencji. Na przykład nie musisz synchronizować tej sekwencji:

synchronizedMap.put(key1, valuesList1);
synchronizedMap.put(key2, valuesList2);

Tutaj

synchronizedMap.put(key2, valuesList2);

wywołanie metody nie opiera się na wynikach poprzedniego

synchronizedMap.put(key1, valuesList1);

wywołanie metody (nie ma znaczenia, czy jakiś wątek ingerował między dwoma wywołaniami metody i na przykład usunął key1).

Wydaje mi się to poprawne. Gdybym miał coś zmienić, przestałbym używać Collections.synchronizedMap () i zsynchronizował wszystko w ten sam sposób, tylko po to, aby było jaśniej.

Również wymieniłbym

  if (synchronizedMap.containsKey(key)) {
    synchronizedMap.get(key).add(value);
  }
  else {
    List<String> valuesList = new ArrayList<String>();
    valuesList.add(value);
    synchronizedMap.put(key, valuesList);
  }

z

List<String> valuesList = synchronziedMap.get(key);
if (valuesList == null)
{
  valuesList = new ArrayList<String>();
  synchronziedMap.put(key, valuesList);
}
valuesList.add(value);

Sposób synchronizacji jest prawidłowy. Ale jest w tym haczyk

1. Zsynchronizowana otoka dostarczana przez platformę Collection zapewnia, że ​​wywołania metody, tj. Add / get / contains, będą działać wzajemnie wykluczające się.

Jednak w prawdziwym świecie generalnie przesyła się zapytanie do mapy przed wprowadzeniem wartości. Dlatego musiałbyś wykonać dwie operacje, a zatem potrzebny jest zsynchronizowany blok. Więc sposób, w jaki go użyłeś, jest poprawny. Jednak.

2. Mogłeś użyć współbieżnej implementacji Map dostępnej w ramach kolekcji. Zaletą „ConcurrentHashMap” jest

za. Ma API „putIfAbsent”, które robi to samo, ale w bardziej efektywny sposób.

b. Jego wydajność: dThe CocurrentMap po prostu blokuje klucze, dzięki czemu nie blokuje całego świata mapy. Gdzie jak masz zablokowane klucze i wartości.

do. Mogłeś przekazać odniesienie do swojego obiektu mapy gdzie indziej w swojej bazie kodu, gdzie ty / inny programista w twoim teanie może skończyć się nieprawidłowym użyciem go. Tzn. Może po prostu dodawać () lub pobierać () bez blokowania obiektu mapy. Dlatego jego wywołanie nie będzie działało wzajemnie wyłączając się z blokiem synchronizacji. Jednak korzystanie z implementacji współbieżnej daje pewność, że nigdy nie będzie można jej nieprawidłowo użyć / zaimplementować.

Sprawdź Kolekcje Google " Multimap, np stronie 28 tej prezentacji .

Jeśli z jakiegoś powodu nie możesz użyć tej biblioteki, rozważ użycie ConcurrentHashMapzamiast SynchronizedHashMap; ma sprytną putIfAbsent(K,V)metodę, za pomocą której możesz atomowo dodać listę elementów, jeśli jeszcze jej tam nie ma. Rozważ także użycie CopyOnWriteArrayListwartości mapy, jeśli pozwalają na to twoje wzorce użytkowania.