[Concurrence][Synchronisation Java] Faut-il synchroniser cet attribut ?

Bonjour à tous,

Soit l’attribut end de la classe A ; il est de type boolean.

La classe A hérite de Thread et fait usage de son attribut end dans un while, comme suit :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10

class A extends Thread {
   private boolean end;
   public A() {
      this.end = false;
   }

   public void run() {
      while(!this.end) { ... }
   }
}

Cette même classe propose également un setter (méthode end()) pour ré-écrire cet attribut :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14

class A extends Thread {
   private boolean end;
   A() {
      this.end = false;
   }

   public void run() {
      while(!this.end) { ... }
   }

   void end() {
      this.end = true;
   }
}

Une classe B, qui n’hérite pas de Thread mais qui contient la méthode public static void main(String[] args) ET QUI DONC est le thread principal, fait appel au setter de la classe A :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11

class B {

   public static void main(String[] args) {
      ...
      A a = new A();
      ...
      a.end();
      ...
   }

}

Ma question est : faut-il synchroniser le setter end() de la classe A ?

Autrement dit : est-il possible que le thread de la classe A évalue la valeur de l’attribut end dans son while et qu’"en même temps" (bon c’est jamais réellement simultané mais passons), le thread principal (classe B) modifie cette valeur ? On aurait alors un problème de concurrence. Et donc il faudrait effectivement mettre en place une synchronisation.

Qu’en dites-vous ?

Merci d’avance.

Oui mais le prof ne semble pas du tout faire attention à ce "détail"… Mais en en tenant compte, ça donnerait quoi concrètement avec mon code ? Il y aurait une troisième classe qui contiendrait les synchronized ? Au final je ne vois pas trop ce que ça changerait =/

Sinon, juste pour être sûr : du coup je remplace le while(!end) par un while(!this.getEnd()), je crée ce getter : synchronized boolean getEnd() { return this.end; } et je modifie le setter en : synchronized void end() { this.end = true; } ? Enfin, dans le main thread, je garde mon a.end(); ?

Hello,

Il faut voir synchronized non pas comme un mot-clé magique, mais comme un endroit dans le code où tu dis : "s’il y a quelqu’un dans ce bout de code, tu attends, sinon tu peux passer". Du coup ici tes synchronized ne sont pas utiles puisque getEnd est accédé par une thread et setEnd une autre thread.

Par contre ici, ton champ end mérite d’être déclaré volatile pour t’assurer de bien aller le lire en RAM et que sa vraie valeur n’est pas dans le cache du processeur.

Note concernant synchronized: il faut éviter de l’utiliser dans la signature d’une méthode. En effet, les deux méthodes suivantes sont équivalentes :

1
2
3
4
5
6
7
8
9

public synchronized void doSomeStuff() {
    // do some stuff...
}

public void doSomeStuff() {
    synchronized(this) {
        // do some stuff...
    }
}

Or il vaut mieux utiliser synchronized sur des objets privés et inaccessibles à l’extérieur de ta classe pour deux raisons :

• si tu as besoin d’utiliser plusieurs locks, il n’est pas cohérent d’avoir this et d’autres objets arbitraires pour cela (autant n’utiliser que des objets arbitraires)
• si une classe extérieure, pour une raison inconnue, décide d’utiliser ta classe en tant que lock, cela va interférer avec ta logique de synchronisation au sein de ta classe

Deuxième note : essaie d’éviter d’hériter de Thread, il est préférable que tu utilises le constructeur public Thread(Runnable r).

Au final, ton code devrait ressembler à ceci :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

class A implements Runnable {
    private volatile boolean end = false; 

    @Override
    public void run() {
        while(!end) {
            // Do some stuff
        }
    }

    public void end() {
        end = true;
    }
}

class B {
    public static void main(String[] args) {
        A a = new A();
        Thread t = new Thread(a);
        t.start();
        // Do some stuff
        a.end();
    }
}

En espérant t’avoir aidé !

Je ne comprends pas malgré tes explications pourquoi tu as enlevé la synchro du end ?

Edit : pour compléter ma réponse, le verrou de ces deux méthodes synchronisées est le même (l’objet courant). Donc le thread qui est dans le getter empêchera l’autre d’accéder au setter en même temps. Ainsi avons-nous protégé la ressource. Non ?

Pour moi, avec le code en l’état (sans blocs synchronized), ton objet ne peut pas être dans un état incohérent quel que soit "l’endroit" où un thread s’est arrêté (notamment parce qu’une assignation d’un boolean, qui plus est volatile, est atomique). La seule chose qui t’intéresse, c’est de récupérer la valeur "la plus récente" du champ end, donc celle qui a été mis à jour par la thread principale, d’où la déclaration volatile.

Logiquement, si tu laisses les synchronized sur tes getter et setter, ça marche hein (j’ai pas testé par contre). Cependant, un simple volatile fait le même travail en plus élégant et ça t’évite de faire attendre tes threads inutilement.

D’acc d’acc, mais je ne peux pas utiliser volatile (contrainte du TP) ; nous sommes d’accord pour dire que sans, il faut bien utiliser mes synchronized non ?