Pourquoi cette classe n'est-elle pas thread-safe?

94
class ThreadSafeClass extends Thread
{
     private static int count = 0;

     public synchronized static void increment()
     {
         count++;
     }

     public synchronized void decrement()
     {
         count--;
     }
}

Quelqu'un peut-il expliquer pourquoi la classe ci-dessus n'est pas thread-safe?

das kinder
la source
6
Je ne sais pas pour Java, mais il semble que chacune de ces méthodes soit individuellement thread-safe, mais vous pourriez avoir un thread dans chacune des méthodes en même temps. Peut-être que si vous avez une seule méthode qui prend un bool ( increment), ce serait thread-safe. Ou si vous avez utilisé un objet de verrouillage. Comme je l'ai dit, je ne connais pas Java - mon commentaire découle de la connaissance du C #.
Wai Ha Lee
Je ne connais pas très bien Java non plus, mais pour synchroniser l'accès à une variable statique, le synchronizeddoit être utilisé uniquement dans les méthodes statiques. Donc, à mon avis, même si vous supprimez la incrementméthode, ce n'est toujours pas threadsafe puisque deux instances (qui n'ont qu'un accès synchronisé via la même instance) peuvent appeler la méthode simultanément.
Onur
4
Il est thread-safe tant que vous ne créez jamais d'instance de la classe.
Benjamin Gruenbaum
1
Pourquoi pensez-vous que ce n'est pas thread-safe?
Raedwald

Réponses:

134

Étant donné que la incrementméthode est, staticelle se synchronisera sur l'objet de classe pour le ThreadSafeClass. La decrementméthode n'est pas statique et se synchronisera sur l'instance utilisée pour l'appeler. C'est-à-dire qu'ils se synchroniseront sur des objets différents et ainsi deux threads différents pourront exécuter les méthodes en même temps. Étant donné que les opérations ++et --ne sont pas atomiques, la classe n'est pas thread-safe.

De plus, comme countest static, en le modifiant à partir de decrementlaquelle est synchronisée instance procédé est dangereux car il peut être demandé à différentes instances et modifier en countmême temps que de cette façon.

K Erlandsson
la source
12
Vous pouvez ajouter, puisque countc'est static, avoir une méthode d'instance decrement()est incorrect, même s'il n'y avait pas de static increment()méthode, car deux threads peuvent invoquer decrement()sur des instances différentes modifiant le même compteur simultanément.
Holger
1
C'est peut-être une bonne raison de préférer utiliser des synchronizedblocs en général (même sur tout le contenu de la méthode) au lieu d'utiliser le modificateur sur la méthode, c'est synchronized(this) { ... }-à- dire et synchronized(ThreadSafeClass.class) { ... }.
Bruno
++et --ne sont pas atomiques, même survolatile int . Synchronizedprend en charge le problème de lecture / mise à jour / écriture avec ++/ --, mais le staticmot-clé est, eh bien, clé ici. Bonne réponse!
Chris Cirefice
Re, modifier [un champ statique] depuis ... une méthode d'instance synchronisée est faux : il n'y a rien de mal en soi à accéder à une variable statique depuis une méthode d'instance, et il n'y a rien de mal en soi à y accéder depuis une synchronizedméthode d'instance non plus. Ne vous attendez pas à ce que "synchronisé" sur la méthode d'instance fournisse une protection aux données statiques. Le seul problème ici est ce que vous avez dit dans votre premier paragraphe: les méthodes utilisent des verrous différents pour tenter de protéger les mêmes données, et cela ne fournit bien sûr aucune protection.
Solomon Slow
23

Vous avez deux méthodes synchronisées, mais l'une d'elles est statique et l'autre non. Lors de l'accès à une méthode synchronisée, en fonction de son type (statique ou non statique), un objet différent sera verrouillé. Pour une méthode statique, un verrou sera placé sur l'objet Class, tandis que pour le bloc non statique, un verrou sera placé sur l'instance de la classe qui exécute la méthode. Étant donné que vous avez deux objets verrouillés différents, vous pouvez avoir deux threads qui modifient le même objet simultanément.

Slimu
la source
14

Quelqu'un peut-il expliquer pourquoi la classe ci-dessus n'est pas thread-safe?

  • increment étant statique, la synchronisation se fera sur la classe elle-même.
  • decrementn'étant pas statique, la synchronisation se fera sur l'instanciation de l'objet, mais cela ne sécurise rien car countc'est statique.

J'aimerais ajouter que pour déclarer un compteur thread-safe, je pense que le moyen le plus simple est d'utiliser à la AtomicIntegerplace d'un int primitif.

Laissez-moi vous rediriger vers le java.util.concurrent.atomicpackage-info.

Jean-François Savard
la source
7

Les réponses des autres sont assez bonnes expliquées la raison. J'ajoute juste quelque chose pour résumer synchronized:

public class A {
    public synchronized void fun1() {}

    public synchronized void fun2() {}

    public void fun3() {}

    public static synchronized void fun4() {}

    public static void fun5() {}
}

A a1 = new A();

synchronizedactivé fun1et fun2est synchronisé au niveau de l'objet d'instance. synchronizedon fun4est synchronisé au niveau de l'objet de classe. Ce qui signifie:

  1. Lorsque 2 threads appellent a1.fun1()en même temps, ce dernier appel sera bloqué.
  2. Lorsque le fil 1 a1.fun1()et le fil 2 appellent a1.fun2()en même temps, ce dernier appel sera bloqué.
  3. Lorsque le thread 1 appelle a1.fun1()et le thread 2 appelle a1.fun3()en même temps, pas de blocage, les 2 méthodes seront exécutées en même temps.
  4. Lorsque le thread 1 appelle A.fun4(), si d'autres threads appellent A.fun4()ou A.fun5()en même temps, ces derniers appels seront bloqués car synchronizedon fun4est au niveau de la classe.
  5. Lorsque le thread 1 appelle A.fun4(), le thread 2 appelle a1.fun1()en même temps, pas de blocage, les 2 méthodes seront exécutées en même temps.
coderz
la source
6
  1. decrementse verrouille sur une chose différente incrementpour qu'ils ne s'empêchent pas de fonctionner.
  2. L'appel decrementsur une instance se verrouille sur une chose différente de l'appel decrementsur une autre instance, mais ils affectent la même chose.

Le premier signifie que le chevauchement des appels à incrementet decrementpourrait entraîner une annulation (correcte), un incrément ou une diminution.

Le second signifie que deux appels qui se chevauchent decrementsur des instances différentes peuvent entraîner un double décrément (correct) ou un seul décrément.

Jon Hanna
la source
4

Étant donné que deux méthodes différentes, l'une est au niveau de l'instance et l'autre au niveau de la classe, vous devez donc verrouiller 2 objets différents pour le rendre ThreadSafe

jaleel_quest
la source
1

Comme expliqué dans d'autres réponses, votre code n'est pas thread-safe car la méthode statique increment()verrouille le moniteur de classe et la méthode non statique decrement()verrouille le moniteur d'objets.

Pour cet exemple de code, une meilleure solution existe sans synchronzedutilisation de mot-clé. Vous devez utiliser AtomicInteger pour assurer la sécurité des threads.

Thread safe en utilisant AtomicInteger:

import java.util.concurrent.atomic.AtomicInteger;

class ThreadSafeClass extends Thread {

    private static AtomicInteger count = new AtomicInteger(0);

    public static void increment() {
        count.incrementAndGet();
    }

    public static void decrement() {
        count.decrementAndGet();
    }

    public static int value() {
        return count.get();
    }

}
Ravindra babu
la source