Java synchronisierter Block vs. Collections.synchronizedMap

84

Ist der folgende Code so eingerichtet, dass die Anrufe korrekt synchronisiert werden 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);
      }
    }
  }
}

Nach meinem Verständnis benötige ich den synchronisierten Block addToMap(), um zu verhindern, dass ein anderer Thread aufruft remove()oder containsKey()bevor ich den Aufruf erhalte, put()aber ich benötige keinen synchronisierten Block, doWork()da ein anderer Thread den synchronisierten Block addToMap()vor der remove()Rückkehr nicht eingeben kann, da ich die Map ursprünglich erstellt habe mit Collections.synchronizedMap(). Ist das korrekt? Gibt es einen besseren Weg, dies zu tun?

Ryan Ahearn
quelle

Antworten:

90

Collections.synchronizedMap() garantiert, dass jede atomare Operation, die Sie auf der Karte ausführen möchten, synchronisiert wird.

Das Ausführen von zwei (oder mehr) Operationen auf der Karte muss jedoch in einem Block synchronisiert werden. Also ja - Sie synchronisieren richtig.

Yuval Adam
quelle
26
Ich denke, es wäre gut zu erwähnen, dass dies funktioniert, da die Javadocs explizit angeben, dass synchronizedMap auf der Karte selbst synchronisiert wird und keine interne Sperre. Wenn dies der Fall wäre, wäre synchronized (synchronizedMap) nicht korrekt.
extraneon
2
@ Yuval könnten Sie Ihre Antwort etwas ausführlicher erklären? Sie sagen, sychronizedMap führt Operationen atomar aus, aber warum sollten Sie dann jemals einen eigenen synchronisierten Block benötigen, wenn die syncMap alle Ihre Operationen atomar macht? Ihr erster Absatz scheint es auszuschließen, sich um den zweiten zu sorgen.
Almel
@almel siehe meine Antwort
Sergey
2
Warum muss ein synchronisierter Block vorhanden sein, da die Karte bereits verwendet wird Collections.synchronizedMap()? Ich verstehe den zweiten Punkt nicht.
Bimal Sharma
15

Wenn Sie JDK 6 verwenden, sollten Sie ConcurrentHashMap ausprobieren

Beachten Sie die putIfAbsent-Methode in dieser Klasse.

TofuBeer
quelle
9
Es ist tatsächlich JDK 1.5
Bogdan
13

Es besteht die Möglichkeit eines subtilen Fehlers in Ihrem Code.

[ UPDATE: Da er map.remove () verwendet, ist diese Beschreibung nicht vollständig gültig. Ich habe diese Tatsache beim ersten Mal verpasst. :( Vielen Dank an den Autor der Frage, der darauf hingewiesen hat. Ich lasse den Rest unverändert, habe aber die Lead-Anweisung dahingehend geändert, dass möglicherweise ein Fehler vorliegt.]

In doWork () erhalten Sie den Listenwert threadsicher aus der Map. Danach greifen Sie jedoch in einer unsicheren Angelegenheit auf diese Liste zu. Beispielsweise kann ein Thread die Liste in doWork () verwenden, während ein anderer Thread synchronizedMap.get (Schlüssel) .add (Wert) in addToMap () aufruft . Diese beiden Zugriffe sind nicht synchronisiert. Als Faustregel gilt, dass sich die thread-sicheren Garantien einer Sammlung nicht auf die von ihnen gespeicherten Schlüssel oder Werte erstrecken.

Sie können dies beheben, indem Sie eine synchronisierte Liste wie in die Karte einfügen

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

Alternativ können Sie auf der Karte synchronisieren, während Sie auf die Liste in doWork () zugreifen :

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

Die letzte Option wird die Parallelität ein wenig einschränken, ist aber IMO etwas klarer.

Außerdem ein kurzer Hinweis zu ConcurrentHashMap. Dies ist eine wirklich nützliche Klasse, aber nicht immer ein geeigneter Ersatz für synchronisierte HashMaps. Zitat aus seinen Javadocs,

Diese Klasse ist in Programmen, die auf der Thread-Sicherheit, jedoch nicht auf den Synchronisationsdetails beruhen, vollständig mit Hashtable kompatibel .

Mit anderen Worten, putIfAbsent () eignet sich hervorragend für atomare Einfügungen, garantiert jedoch nicht, dass sich andere Teile der Karte während dieses Aufrufs nicht ändern. es garantiert nur Atomizität. In Ihrem Beispielprogramm verlassen Sie sich bei anderen Dingen als put () auf die Synchronisationsdetails von (einer synchronisierten) HashMap.

Letztes Ding. :) Dieses großartige Zitat aus Java Concurrency in der Praxis hilft mir immer beim Entwerfen eines Debugging-Multithread-Programms.

Für jede veränderbare Statusvariable, auf die mehr als ein Thread zugreifen kann, müssen alle Zugriffe auf diese Variable mit derselben Sperre ausgeführt werden.

JLR
quelle
Ich sehe Ihren Standpunkt zum Fehler, wenn ich mit synchronizedMap.get () auf die Liste zugegriffen habe. Sollte das nächste Hinzufügen mit diesem Schlüssel nicht eine neue ArrayList erstellen und nicht die in doWork verwendete stören, da ich remove () verwende?
Ryan Ahearn
Richtig! Ich bin total an deiner Entfernung vorbeigekommen.
JLR
1
Für jede veränderbare Statusvariable, auf die mehr als ein Thread zugreifen kann, müssen alle Zugriffe auf diese Variable mit derselben Sperre ausgeführt werden. ---- Ich füge im Allgemeinen eine private Eigenschaft hinzu, die nur ein neues Object () ist, und verwende diese für meine Synchronisierungsblöcke. Auf diese Weise weiß ich alles durch das Rohe für diesen Kontext. synchronisiert (objectInVar) {}
AnthonyJClink
11

Ja, Sie synchronisieren richtig. Ich werde dies genauer erklären. Sie müssen zwei oder mehr Methodenaufrufe für das synchronizedMap-Objekt nur dann synchronisieren, wenn Sie sich auf die Ergebnisse vorheriger Methodenaufrufe im nachfolgenden Methodenaufruf in der Reihenfolge der Methodenaufrufe für das synchronizedMap-Objekt verlassen müssen. Schauen wir uns diesen Code an:

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

In diesem Code

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

und

synchronizedMap.put(key, valuesList);

Methodenaufrufe basieren auf dem Ergebnis des vorherigen

synchronizedMap.containsKey(key)

Methodenaufruf.

Wenn die Reihenfolge der Methodenaufrufe nicht synchronisiert wurde, ist das Ergebnis möglicherweise falsch. Beispiel thread 1: Ausführen der Methode addToMap()und thread 2Ausführen der Methode doWork() Die Reihenfolge der Methodenaufrufe für das synchronizedMapObjekt kann wie folgt sein: Thread 1hat die Methode ausgeführt

synchronizedMap.containsKey(key)

und das Ergebnis ist " true". Nachdem dieses Betriebssystem die Ausführungssteuerung auf umgeschaltet thread 2und ausgeführt hat

synchronizedMap.remove(key)

Danach wurde die Ausführungssteuerung wieder auf die umgeschaltet thread 1und zum Beispiel ausgeführt

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

zu glauben, dass das synchronizedMapObjekt das enthält keyund NullPointerExceptiongeworfen wird, weil synchronizedMap.get(key) es zurückkehren wird null. Wenn die Reihenfolge der Methodenaufrufe für das synchronizedMapObjekt nicht von den Ergebnissen der anderen abhängig ist, müssen Sie die Reihenfolge nicht synchronisieren. Zum Beispiel müssen Sie diese Sequenz nicht synchronisieren:

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

Hier

synchronizedMap.put(key2, valuesList2);

Der Methodenaufruf basiert nicht auf den Ergebnissen des vorherigen

synchronizedMap.put(key1, valuesList1);

Methodenaufruf (es ist egal, ob ein Thread zwischen den beiden Methodenaufrufen eingegriffen und beispielsweise den entfernt hat key1).

Sergey
quelle
4

Das sieht für mich richtig aus. Wenn ich etwas ändern würde, würde ich die Verwendung von Collections.synchronizedMap () einstellen und alles auf die gleiche Weise synchronisieren, um es klarer zu machen.

Auch würde ich ersetzen

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

mit

List<String> valuesList = synchronziedMap.get(key);
if (valuesList == null)
{
  valuesList = new ArrayList<String>();
  synchronziedMap.put(key, valuesList);
}
valuesList.add(value);
Paul Tomblin
quelle
3
Die Sache zu tun. Ich verstehe nicht, warum wir die Collections.synchronizedXXX()APIs verwenden sollten, wenn wir noch ein Objekt (in den meisten Fällen nur die Sammlung selbst) in der Logik unserer täglichen App
synchronisieren müssen
3

Die Art und Weise, wie Sie synchronisiert haben, ist korrekt. Aber es gibt einen Haken

  1. Der vom Collection Framework bereitgestellte synchronisierte Wrapper stellt sicher, dass die Methodenaufrufe Ie add / get / includes sich gegenseitig ausschließen.

In der realen Welt würden Sie jedoch im Allgemeinen die Karte abfragen, bevor Sie den Wert eingeben. Daher müssten Sie zwei Operationen ausführen, und daher wird ein synchronisierter Block benötigt. Die Art und Weise, wie Sie es verwendet haben, ist also korrekt. Jedoch.

  1. Sie hätten eine gleichzeitige Implementierung von Map verwenden können, die im Collection-Framework verfügbar ist. Der Vorteil von 'ConcurrentHashMap' ist

ein. Es hat eine API 'putIfAbsent', die das gleiche tun würde, aber effizienter.

b. Es ist effizient: dDie CocurrentMap sperrt nur die Schlüssel, wodurch nicht die gesamte Welt der Karte blockiert wird. Wo Sie Schlüssel sowie Werte gesperrt haben.

c. Möglicherweise haben Sie die Referenz Ihres Kartenobjekts an einer anderen Stelle in Ihrer Codebasis übergeben, wo Sie / andere Entwickler in Ihrer Tean sie möglicherweise falsch verwenden. Das heißt, er kann einfach alle hinzufügen () oder erhalten (), ohne das Objekt der Karte zu sperren. Daher schließt sich sein Aufruf nicht gegenseitig für Ihren Synchronisierungsblock aus. Die Verwendung einer gleichzeitigen Implementierung gibt Ihnen jedoch die Gewissheit, dass sie niemals falsch verwendet / implementiert werden kann.

Jai Pandit
quelle
2

Schauen Sie sich Google Collections an Multimap, z. B. Seite 28 dieser Präsentation .

Wenn Sie diese Bibliothek aus irgendeinem Grund nicht verwenden können, sollten Sie sie ConcurrentHashMapanstelle von verwenden SynchronizedHashMap. Es gibt eine raffinierte putIfAbsent(K,V)Methode, mit der Sie die Elementliste atomar hinzufügen können, wenn sie noch nicht vorhanden ist. Erwägen Sie auch die Verwendung CopyOnWriteArrayListfür die Kartenwerte, wenn Ihre Verwendungsmuster dies rechtfertigen.

Barend
quelle