Discussion :

[alladdin]

Bonjour,

J'ai lu ce tutoriel sur le singleton en environnement multithread : http://christophej.developpez.com/tu...n/multithread/

Dedans on trouve notamment l'explication du pourquoi le DCL ne marche pas (possibilite d'avoir un objet partiellement initialise). Je pense avoir trouve une solution a ce problème et je voudrais avoir votre avis concernant cette solution.

L'idée est de rajouter , au niveau de la classe, un boolean (isCompleted) pour savoir si l'exécution du constructeur est fini ou non. Si le constructeur a fini, on retourne l'objet sinon on attend. Voici un code descriptif :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
import java.util.* ;
class Singleton{
	private static Singleton instance;
	private List maList;
	private static boolean isCompleted = false;
	private Singleton(){
		maList= new ArrayList();
	}
	public static Singleton getInstance(){
	   if (instance==null){
		synchronized (Singleton.class){			
			if(instance==null){			
				instance=new Singleton();
                                isCompleted = true;
	                 }
	        }
	    } else{
                 while(!isCompleted){
                        wait(2000); //ajouter le try catch block. 2000 est un exemple mettre le temps approximatif de construction de l'objet 
                 }
            }
	 return instance;				
	}
}

A votre avis est ce que cette méthode marche ou pas ? Si oui qu'en pensez vous de manière général (meilleur que synchroniser toute la méthode getInstance() ?).


Merci d'avance pour vos réponses .

[adiGuba]

Salut,



Ca peut fonctionner... mais c'est lourd !

La plupart du temps l'initialisation static est amplement suffisante et bien plus simple à mettre en place...


a++

[alladdin]

Merci pour ta réponse rapide. Je ne peux pas utiliser l'initialiseur static, car j'ai absolument besoin que l'objet soit construit au premier appel de la méthode getInstance et pas avant. J'ai donc le choix entre synchroniser toute la méthode ou faire ce DCL "amélioré" (je trouve personnellement la méthode du ThreadLocal trop complique).

[tchize_]

et utiliser des apis comme commons discovery? Qui sont déjà prévue et conçues pour éviter ce problème

[alladdin]

Je ne connais pas l'api commons discovery, je vais faire un tour sur le net pour en apprendre d'avantage. En tout cas merci pour ce pointeur.

Sinon je pense que le code que tu as propose est plus simple que le mien et je pense qu'il devrait aussi fonctionner. Si ce n'est pas le cas quel est le problème ?

Encore merci pour ton aide.

[tchize_]

je l'ai retiré, mon code se faisait avoir comme un bleu: le out of order write

[adiGuba]

Merci pour ta réponse rapide. Je ne peux pas utiliser l'initialiseur static, car j'ai absolument besoin que l'objet soit construit au premier appel de la méthode getInstance et pas avant.

Si getInstance() est l'unique élément static et que les constructeurs sont bien privés, alors ce sera bien le cas, à moins de charger la classe explicitement via un Class.forName("")...


a++

[thelvin]

Notons au passage que dans le code donné au départ, isCompleted n'est pas volatile.
Par conséquent, si elle est false en entrant dans la boucle while, elle ne deviendra jamais true à l'intérieur de cette boucle, et la boucle ne finira pas.

volatile permettrait à sa valeur de changer pendant la boucle, et avec les JVM modernes cela règlerait en effet le problème... Mais du coup, je ne vois pas ce que cela est censé faire de mieux qu'un DCL ?

[alladdin]

Merci adiGuba pour ta réponse. Dans ce cas je pense pouvoir l'utiliser .

@ tchize_
Je remets le code que t'as écris pour pouvoir en rediscuter (j'espère que ca te gêne pas), mais je ne vois pas pourquoi on aurais un problème de "out of order write" car la variable isCompleted utilise le même principe que l'initialiseur static non ? Du coup on est sur que si l'objet est pas complètement fini on va dans le bloc syncronized et donc ca devrait être bon, non ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
 
public static Singleton getInstance(){
	   if (!isCompleted){
		synchronized (Singleton.class){			
			if(!isCompleted){		// ou if(instance==null) devrait revenir au meme ici non ? 	
				instance=new Singleton();
                                isCompleted = true;
	                 }
	        }
	    } 
	 return instance;				
	}

Merci encore a vous de m'aider a comprendre les mécanismes du langage java .

[tchize_]

de la même manière que instance peut être non null avant que le constructeur soit appelé, la jvm peut réordonnancer les écriture pour arriver à cet ordre d'exécution:

• constructeur
• stockage isComplete à true
• stockage instance à objet créé avant

ce qui amènerais dans certains cas limite à ce que isComplete est à true, le constructeur à été appelé, mais instance est toujours null. Donc getInstance() renverra null dans ce cas

[tchize_]

Notons au passage que dans le code donné au départ, isCompleted n'est pas volatile.
Par conséquent, si elle est false en entrant dans la boucle while, elle ne deviendra jamais true à l'intérieur de cette boucle, et la boucle ne finira pas.

Je ne suis pas certains que ce soit nécessaire dans ce cas ci. En effet, il y a un appel de méthode dans la boucle, la jvm n'a aucun moyen de savoir si cet appel ne change pas le champ de manière synchrone et donc est bien forcée de le relire depuis les champs de la classe.

Par contre, ta réflexion aurait été totalement justifié si on avait eu une boucle active:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

while(!isComplete){}

Enfin, rappelons pour le fun que volatile peut être ignoré par la JVM

[alladdin]

ce out of order write fait vraiment peur

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
 
Object obj = new Object();
if (obj == null){
// possible d'avoir ce cas " grâce" au out of order write ???
}

[tchize_]

non, parce que tu reste dans le même thread. Le out of order write permet à la jvm de réordonnancer des écriture, pour autant que celle-ci n'affectent pas l'algorithme en question pour autant que la synchronisation y soit correctement gérée. Ce n'est donc pas visible. Sauf quand tu va tripatouiller les variable en ne faisant pas de synchro, comme dans ton cas lorsque ton deuxième thread va lire isComplete puis instance sans synchronisation .

[alladdin]

ok donc si je comprends bien ce code ci devrait être correct :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
public static Singleton getInstance(){
	   if (!isCompleted){
		synchronized (Singleton.class){			
			if(!isCompleted){		
				instance=new Singleton();
                                if(instance != null){ // force a ce que le stockage "instance" soit fini avant de continuer l'execution du programme. 
                                 isCompleted = true;
                                }
	                 }
	        }
	    } 
	 return instance;				
	}

[tchize_]

Absolument pas, tu peux très bien avoir traversé ton if avec succès et le stockage n'a pas encore eu lieu. Au sein d'une méthode, les valeur des champs de classe peuvent être mis en cache dans les registres, puisque toutes les opérations passent par les registres, et donc le if peut très bien ne regarder que ce qui est en registre.
La règle est simple, quand tu accède depuis un bloc non synchrone à des données modifiées par un autre thread, tu n'a aucun garantie sur l'état de ces données. Seul une exclusion mutuelle par des blocs synchronized le permet.

[thelvin]

Je ne suis pas certains que ce soit nécessaire dans ce cas ci. En effet, il y a un appel de méthode dans la boucle, la jvm n'a aucun moyen de savoir si cet appel ne change pas le champ de manière synchrone et donc est bien forcée de le relire depuis les champs de la classe.

Par contre, ta réflexion aurait été totalement justifié si on avait eu une boucle active:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

while(!isComplete){}

Voilà. C'est là que j'avais déconnecté .

[bulbo]

Bon j'arrive après la bagarre, mais je me souviens d'une version qui avait été proposée (par Adiguba je crois bien) lors d'une discussion animée autour du Singleton thread-safe avec lazy loading.

Ca donnait un truc comme ça et de ce que j'avais compris, cela résolvait parfaitement le problème:

Code java :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
public class MaClasse {
 
    private static class InstanceHolder {
 
        private InstanceHolder() {
        }
 
        static MaClasse instance = new MaClasse();
    }
 
    public static MaClasse getInstance() {
        return InstanceHolder.instance;
    }
 
   private MaClasse() { }
 ....

Si ce n'est pas le cas .. j'espère qu'une bonne âme aura l'amabilité de me détromper .. que j'arrête d'utiliser ça partout

Bulbo

[adiGuba]

Je ne me souviens pas avoir posté cela... mais le principe est le même que l'initialisation statique :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
class Singleton {
	private static final Singleton INSTANCE = new Singleton();
 
	public static Singleton getInstance() {
		return INSTANCE;
	}
}

La principale différence étant qu'on est sûr que l'instance du Singleton est bien au premier appel de getInstance() et non pas avant, même si la classe Singleton comporte d'autre éléments static ou qu'on force son chargement par un moyen quelconque...



a++

[alladdin]

Absolument pas, tu peux très bien avoir traversé ton if avec succès et le stockage n'a pas encore eu lieu. Au sein d'une méthode, les valeur des champs de classe peuvent être mis en cache dans les registres, puisque toutes les opérations passent par les registres, et donc le if peut très bien ne regarder que ce qui est en registre.
La règle est simple, quand tu accède depuis un bloc non synchrone à des données modifiées par un autre thread, tu n'a aucun garantie sur l'état de ces données. Seul une exclusion mutuelle par des blocs synchronized le permet.

Désolé d'insister, mais j'essaie juste de comprendre comment tout ca marche.
Qu'est ce qui fait que l'implémentation du premier post marche ? Pourquoi on ne peut avoir le même cas (isCompleted est stocker a true ce qui fait sortir de la boucle while mais instance n'a toujours pas été stocke donc return null).

Sinon autre petit question au passage, supposons que dans ma classe singleton j'ai un champ int total = 0; J'ai un getter et un "incrementeur" sur ce champ total. Pour pas avoir de concurrence d'accès la méthode "increment" est protégé par un verrou. Si je comprends bien ce que tu me dis je dois aussi protégé la méthode get sinon je risque de ne pas avoir la dernière valeur de total ? (le thread qui appel le get et le thread qui appel increment sont differents, le thread increment a "normalement" fini sa tache).


Merci pour ton aide.

[tchize_]

Pourquoi on ne peut avoir le même cas (isCompleted est stocker a true ce qui fait sortir de la boucle while mais instance n'a toujours pas été stocke donc return null).

Parce que ce code n'arrivera pas jusqu'au while tant que instance est null

Si je comprends bien ce que tu me dis je dois aussi protégé la méthode get sinon je risque de ne pas avoir la dernière valeur de total ? (le thread qui appel le get et le thread qui appel increment sont differents).

Oui. Enfin, si tu manipule juste le nombre, c'est pas grave. Parce que, au final, que tu vois 4 au lieu de 5 parce que les données n'ont pas encore été stockées, ou que tu vois 4 au lieu de 5 parce t'as appelé le get plsu to que le set, ca change en général rien pour ton code :p

Mais si il y a d'autre structure manipulée dans le synchronized, t'es obligé de synchronized le get pour rester cohérent.


Pour expliquer le out of order, le plus simple est ce code

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
public class Test {
   int a =1;
   int b =2;
   public synchronized change(){
     a=3;
     b=4;
   }
   public display(){
      System.out.println("a = "+a+", b= "+b);
   }
}

Si j'appelle les deux méthodes en parallèle, la JLS me dit que je peux avoir chacun de ces 4 résultats à l'affichage:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
a=1, b=2
a=3, b=2
a=3, b=4
a=1, b=4

alors qu'avec

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
public class Test {
   int a =1;
   int b =2;
   public synchronized change(){
     a=3;
     b=4;
   }
   public synchronized display(){
      System.out.println("a = "+a+", b= "+b);
   }
}

je ne peux avoir que

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
a=1, b=2
a=3, b=4