java thread safe code + une méthode atomique question

Question

J'ai un gestionnaire de classe qui va être accédé par plusieurs threads en même temps, je veux savoir si je l'ai fait de la bonne façon?
aussi je pense que je dois être RemoveFoo atomique, mais je ne suis pas sûr

public class Manager 
{ 
    private ConcurrentHashMap<String, Foo> foos; 
    //init in constructor 

    public RemoveFoo(String name) 
    { 
     Foo foo = foos.Get(name); 
     foo.RemoveAll(); 
     foos.Remove(name); 
    } 

    public AddFoo(Foo foo) 
    {...} 
} 

public class Foo 
{ 
    private Map<String,Bar> bars; 
    //intialize it in constructor 

    //same for RemoveBar 
    public void AddBar(Bar bar) 
    { 
     synchronized(this) 
     { 
      bars.put(bar.id, bar); 
     } 
    } 

    public void RemoveAll() 
    { 
     synchronized(this) 
     { 
      //some before removall logic for each one 
      bars.remove(bar.id, bar); 
     } 
    } 

} 

public class Bar 
{} 

Answer 1

RemoveFoo peut être problématique. Je suggère d'utiliser:

Foo foo = foos.remove (name); 
if (foo != null) foo.removeAll(); 

à la place. Cela garantit que la carte ne change pas entre get() et remove().

En Foo, il suffit de se synchroniser sur bars au lieu de l'instance entière. Mais ce n'est qu'une optimisation mineure.

Answer 2

Déclare RemoveFoo(String) comme synchronized:

public synchronized void RemoveFoo(String name) { 
    … 
} 

Aussi, noter les éléments suivants:

• les noms de méthodes doivent être en minuscules, par ex. removeFoo au lieu de RemoveFoo. Ce n'est pas C#. :)
• Chaque méthode nécessite un type de retour: public removeFoo() n'est pas une déclaration de méthode valide, elle doit être public void removeFoo().

Answer 3

Vous n'avez pas besoin de méthodes synchronisées lorsque vous utilisez un ConcurrentHashMap, sachez toutefois que Foo foo = foos.Get(name) peut renvoyer une valeur nulle car un autre thread a déjà supprimé l'entrée de la carte.

Les membres peuvent être déclarés comme Map<String, Foo> foos, mais doivent être initialsed comme foos = new ConcurrentHashMap<String, Foo>;

Answer 4

Si vous utilisez un ConcurrentHashMap dans Foo comme

private Map<String,Bar> bars = new ConcurrentHashMap<String, Bar>();

vous pouvez peut-être en finir avec la synchronisation Foo ainsi .

Answer 5

Je ne suis pas sûr de ce que vous allez faire sur Foo and Bar, mais cela ressemble à un modèle de désallocation.

Si elles ne sont pas référencées par d'autres, appelez simplement foos.Remove (name); et laissez le moteur GC gérer le désallocation.