Bloc synchronisé Java et Collections.synchronizedMap

Le code suivant est-il configuré pour synchroniser correctement les appels 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);
      }
    }
  }
}

D'après ce que je comprends, j'ai besoin du bloc synchronisé addToMap()pour empêcher un autre thread d'appeler remove()ou containsKey()avant de passer l'appel, put()mais je n'ai pas besoin d'un bloc synchronisé doWork()car un autre thread ne peut pas entrer dans le bloc synchronisé addToMap()avant les remove()retours car j'ai créé la carte à l'origine avec Collections.synchronizedMap(). Est-ce exact? Y a-t-il une meilleure manière de faire cela?

Collections.synchronizedMap() garantit que chaque opération atomique que vous souhaitez exécuter sur la carte sera synchronisée.

L'exécution de deux (ou plus) opérations sur la carte doit cependant être synchronisée dans un bloc. Alors oui, vous synchronisez correctement.

Si vous utilisez JDK 6, vous voudrez peut-être consulter ConcurrentHashMap

Notez la méthode putIfAbsent dans cette classe.

Il existe un potentiel de bogue subtil dans votre code.

[ MISE À JOUR: Comme il utilise map.remove (), cette description n'est pas totalement valide. J'ai raté ce fait la première fois. :( Merci à l'auteur de la question pour l'avoir signalé. Je laisse le reste tel quel, mais j'ai changé l'instruction principale pour indiquer qu'il y a potentiellement un bogue.]

Dans doWork (), vous obtenez la valeur List de la carte de manière thread-safe. Par la suite, cependant, vous accédez à cette liste de manière dangereuse. Par exemple, un thread peut utiliser la liste dans doWork () tandis qu'un autre thread appelle synchronizedMap.get (clé) .add (valeur) dans addToMap () . Ces deux accès ne sont pas synchronisés. La règle de base est que les garanties thread-safe d'une collection ne s'étendent pas aux clés ou aux valeurs qu'elles stockent.

Vous pouvez résoudre ce problème en insérant une liste synchronisée dans la carte comme

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

Vous pouvez également synchroniser sur la carte pendant que vous accédez à la liste dans doWork () :

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

La dernière option limitera un peu la concurrence, mais est un peu plus claire pour l'OMI.

Aussi, une note rapide sur ConcurrentHashMap. C'est une classe vraiment utile, mais qui n'est pas toujours un remplacement approprié pour les HashMaps synchronisés. Citant ses Javadocs,

Cette classe est entièrement interopérable avec Hashtable dans les programmes qui reposent sur sa sécurité de thread mais pas sur ses détails de synchronisation .

En d'autres termes, putIfAbsent () est idéal pour les insertions atomiques mais ne garantit pas que d'autres parties de la carte ne changeront pas pendant cet appel; il ne garantit que l'atomicité. Dans votre exemple de programme, vous vous fiez aux détails de synchronisation d'un HashMap (synchronisé) pour des choses autres que put () s.

Dernière chose. :) Cette excellente citation de Java Concurrency in Practice m'aide toujours à concevoir un débogage de programmes multithreads.

Pour chaque variable d'état mutable accessible par plus d'un thread, tous les accès à cette variable doivent être effectués avec le même verrou maintenu.

Oui, vous synchronisez correctement. J'expliquerai cela plus en détail. Vous devez synchroniser deux ou plusieurs appels de méthode sur l'objet synchronizedMap uniquement dans le cas où vous devez vous fier aux résultats des appels de méthode précédents dans l'appel de méthode suivant dans la séquence d'appels de méthode sur l'objet synchronizedMap. Jetons un coup d'œil à ce code:

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

Dans ce code

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

et

synchronizedMap.put(key, valuesList);

les appels de méthode reposent sur le résultat du précédent

synchronizedMap.containsKey(key)

appel de méthode.

Si la séquence d'appels de méthode n'était pas synchronisée, le résultat pourrait être incorrect. Par exemple, thread 1exécute la méthode addToMap()et thread 2exécute la méthode doWork() La séquence d'appels de méthode sur l' synchronizedMapobjet peut être la suivante: Thread 1a exécuté la méthode

synchronizedMap.containsKey(key)

et le résultat est " true". Une fois que le système d'exploitation a basculé le contrôle d'exécution sur thread 2et qu'il s'est exécuté

synchronizedMap.remove(key)

Une fois que le contrôle d'exécution est revenu sur le thread 1et qu'il s'est exécuté par exemple

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

croyant que l' synchronizedMapobjet contient le keyet NullPointerExceptionsera jeté car synchronizedMap.get(key) retournera null. Si la séquence d'appels de méthode sur l' synchronizedMapobjet ne dépend pas des résultats les uns des autres, vous n'avez pas besoin de synchroniser la séquence. Par exemple, vous n'avez pas besoin de synchroniser cette séquence:

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

Ici

synchronizedMap.put(key2, valuesList2);

l'appel de méthode ne repose pas sur les résultats du précédent

synchronizedMap.put(key1, valuesList1);

appel de méthode (cela ne se soucie pas si un thread a interféré entre les deux appels de méthode et par exemple a supprimé le key1).

Cela me semble correct. Si je devais changer quoi que ce soit, j'arrêterais d'utiliser Collections.synchronizedMap () et je synchroniserais tout de la même manière, juste pour que ce soit plus clair.

Aussi, je remplacerais

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

avec

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

La façon dont vous avez synchronisé est correcte. Mais il ya un hic

1. L'encapsuleur synchronisé fourni par le framework Collection garantit que la méthode appelée add / get / contains s'exécutera mutuellement.

Cependant, dans le monde réel, vous interrogez généralement la carte avant de saisir la valeur. Par conséquent, vous auriez besoin de faire deux opérations et donc un bloc synchronisé est nécessaire. Donc, la façon dont vous l'avez utilisé est correcte. Pourtant.

2. Vous auriez pu utiliser une implémentation simultanée de Map disponible dans le framework Collection. L'avantage de 'ConcurrentHashMap' est

une. Il a une API 'putIfAbsent' qui ferait la même chose mais d'une manière plus efficace.

b. C'est efficace: le CocurrentMap ne fait que verrouiller les clés, donc il ne bloque pas le monde entier de la carte. Où comme vous avez bloqué les clés ainsi que les valeurs.

c. Vous pourriez avoir passé la référence de votre objet map ailleurs dans votre base de code où vous / un autre développeur de votre tean peut finir par l'utiliser de manière incorrecte. C'est-à-dire qu'il peut tout simplement ajouter () ou get () sans verrouiller l'objet de la carte. Par conséquent, son appel ne s'exécutera pas mutuellement à votre bloc de synchronisation. Mais l'utilisation d'une implémentation simultanée vous donne la tranquillité d'esprit qu'elle ne peut jamais être utilisée / implémentée de manière incorrecte.

Consultez Google Collections » Multimap, par exemple la page 28 de cette présentation .

Si vous ne pouvez pas utiliser cette bibliothèque pour une raison quelconque, envisagez d'utiliser à la ConcurrentHashMapplace de SynchronizedHashMap; il a une putIfAbsent(K,V)méthode astucieuse avec laquelle vous pouvez ajouter atomiquement la liste d'éléments si elle n'est pas déjà là. Envisagez également d'utiliser CopyOnWriteArrayListpour les valeurs de mappage si vos modèles d'utilisation le justifient.