[Pattern]PB sur Singleton multi-thread

Version imprimable

Bonjour,

Une petite question j'ai essayé d'implémenter un singleton avec la classe ThreadLocal malheureusement ça ne fonctionne pas correctement: plusieurs instances sont créées quand je lance une multitude de threads

Où est l'erreur. Merci pour vos lumières:

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
public final class BeansPool {
  private static ThreadLocal<BeansPool> _singleton = new ThreadLocal<BeansPool>();
 
  /**
   * @return
   * @throws Exception 
   */
  public final static BeansPool getInstance() throws Exception {
      BeansPool pool = _singleton.get();
 
        if (pool == null) {
            System.out.println("New !!!");
            pool = new BeansPool();
            _singleton.set(pool);
        }
 
        return pool;
  }
}

A+

06/09/2006, 14h36
BugFactory

Citation:

Envoyé par thibaut

plusieurs instances sont créées quand je lance une multitude de threads

http://java.sun.com/j2se/1.4.2/docs/...readLocal.html

Ca a l'air d'être précisément ce à quoi sert ThreadLocal.
06/09/2006, 14h43
thibaut

cf. http://christophej.developpez.com/tu...tithread/#L3.3
06/09/2006, 15h34
divxdede

ThreadLocal est effectivement fait pour associer une valeur "potentiellement" differente à chaque threads de ton application.

Apparamment, ThreadLocal n'est pas synchronisé.
L'instruction
Code:

1 2 private static ThreadLocal<BeansPool> _singleton = new ThreadLocal<BeansPool>();
requière un certain temps et deux threads peuvent l'appeler "simultanément", alors que l'autre n'a pas encore initialisé la variable. La variable est écrasée, mais si entre temps le premier thread a eu le temps de retourner l'objet qu'il a crée ...

Il faut synchroniser l'initialisation des singletons dans un contexte multithreading.
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 public final class BeansPool { private static ThreadLocal<BeansPool> _singleton; /** * @return * @throws Exception */ public final static BeansPool getInstance() throws Exception { synchronized(ThreadLocal.class) { singleton = new ThreadLocal<BeansPool>(); } BeansPool pool = _singleton.get(); if (pool == null) { System.out.println("New !!!"); pool = new BeansPool(); _singleton.set(pool); } return pool; } }

06/09/2006, 19h26
had35
La synchronisation de mes threads n'est pas bonne car, évidemment, il faudrait plutôt quelque chose comme ça :
Code:

1 2 3 4 5 6 synchronized(ThreadLocal.class) { if(singleton == null) singleton = new ThreadLocal<BeansPool>(); } }
Mais l'essentiel est ailleurs. J'ai plutôt l'impression que ThreadLocal fais exactement l'inverse du contrat Singleton, enfin si j'ai bien compris. Il retourne autant d'objet que de thread :aie:

Pourquoi tu utilises ThreadLocal?

Pourquoi ne pas faire simplement:

Code:

1
2
3
4
5
6
7
8
9
10
11
public final class BeanPool {
 
    private static final BeanPool instance = new BeanPool();
 
    private BeanPool() { ... }
 
    public static BeanPool getBeanPool() {
        return instance;
    }
 
}

07/09/2006, 08h35
thibaut

Certes de cette façon ça marche... mais ce n'est pas ce que je veux tester.

Essayes un truc comme ça :

Code:

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
 
public class TestThreadLocal {
	private static ThreadLocal<String> tl = new ThreadLocal<String>();
 
	public String getSingleton() {
		String s = tl.get();
		if(s == null) {
			s = Thread.currentThread().getName();
			tl.set(s);
		}
		return s;
	}
 
	public static void main(String[] args) {
		final TestThreadLocal test = new TestThreadLocal();
		for(int i=0; i<20; i++) {
			new Thread() {
				public void run() {
					System.out.println(test.getSingleton());
					System.out.println(test.getSingleton());
				}
			}.start();
		}
	}
}

07/09/2006, 09h51
thibaut

C'est grosso modo ce que je fais...

Citation:

Envoyé par ®om

Pourquoi tu utilises ThreadLocal?

Pourquoi ne pas faire simplement:

Code:

1
2
3
4
5
6
7
8
9
10
11
public final class BeanPool {
 
    private static final BeanPool instance = new BeanPool();
 
    private BeanPool() { ... }
 
    public static BeanPool getBeanPool() {
        return instance;
    }
 
}

Tu as oublié de synchroniser la méthode static, ce qui est indispensable dans un contexte multithreads ;).

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
public final class BeanPool {

    private static final BeanPool instance = null;

    private BeanPool() { ... }

    public static synchronized BeanPool getBeanPool() {
        if(instance == null) {
            instance = new BeanPool();
        }
        return instance;
    }

}

07/09/2006, 10h10
thibaut

Non c'est faux, car quand tu fais :

Code:

private static final BeanPool instance = new BeanPool();

Il semblerait que dans ce cas tu ne doives pas synchronizer ta méthode.

Testé et approuvé.:roll:
07/09/2006, 10h44
le y@m's

Citation:

Envoyé par thibaut

Non c'est faux, car quand tu fais :

Code:

private static final BeanPool instance = new BeanPool();

Il semblerait que dans ce cas tu ne doives pas synchronizer ta méthode.

Testé et approuvé.:roll:

Effectivement, j'ai lu un peu trop vite :oops: mea culpa.
07/09/2006, 13h58
divxdede

L'initialisation d'un singleton par une instanciation passive n'a pas besoin de synchronizer.

Ensuite tout dépends de ce que tu désires !! un singleton pour l'application ou un singleton par Thread ?
07/09/2006, 14h21
thibaut

Un singleton pour l'application.
07/09/2006, 14h41
had35

Citation:

Envoyé par thibaut

Un singleton pour l'application.

ThreadLocal n'est pas conçu dans cette optique. Comme je voulais le montrer avec le test, et qu'à bien perçu divxdede ;) ,chaque thread obtient son propre singleton.
07/09/2006, 15h02
le y@m's

As tu essayé les deux solutions proposées par ®om et moi-même ?
D'ailleurs, celles-ci sont conseillées dans la conclusion de l'article dont tu as toi-même donné le lien ;)

Citation:

...
il est fortement recommandé d'accepter le coût de la synchronisation de toute la méthode ou d'utiliser un initialiseur static quand votre code le permet.
07/09/2006, 15h10
thibaut

Oui j'utilise la synchronization, mais c'était juste pour tester avec ThreadLocal car l'article http://christophej.developpez.com/tu...tithread/#L3.3

pouvait laisser à penser que l'utilisation de ThreadLocal est une alternative à la synchronization et en fait in n'en est rien (bien que dans d'autres articles les auteurs présentent l'utilisation ThreadLocal comme un bon palliatif au pattern DLC - DoubLock Checking), je suis donc revenu au bon vieux mutex des familles ;).

Citation:

Envoyé par thibaut

Oui j'utilise la synchronization, mais c'était juste pour tester avec ThreadLocal car l'article http://christophej.developpez.com/tu...tithread/#L3.3

pouvait laisser à penser que l'utilisation de ThreadLocal est une alternative à la synchronization et en fait in n'en est rien (bien que dans d'autres articles les auteurs présentent l'utilisation ThreadLocal comme un bon palliatif au pattern DLC - DoubLock Checking), je suis donc revenu au bon vieux mutex des familles ;).

Si tu ne veux ni synchroniser l'ensemble du "getter", et non plus effectuer une initialisation passive, tu peux alors passer par un mutex simple du style
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 public class MySingleton { private static Object lock = new Object(){}; private static MySingleton instance = null; /* constructeurs privés */ ... public static MySingleton getInstance() { synchronized( lock ) { if( instance == null ) instance = new MySingleton(...); } return instance; } }
Mais comme le dit le document, cette solution n'apporte rien en terme de preformance vis a vis d'une syncrhonisation "complete" du getter.
C'est pourquoi on ne conseil que les 2x solutions:
- getter syncrhonisé
- initialisation passive du singleton

Avec une petite preference pour l'initialisation passive.

08/09/2006, 08h33
thibaut

Merci mais je sais tout ça ;)
08/09/2006, 18h59
christopheJ

Je me permet juste de répondre sur la différence entre ton code par rapport au tutoriel.
Comme dit dans le tutoriel et ce sujet, le ThreadLocal gere une variable par thread.

Dans ton code tu as donc un "Singleton" par thread puisque c'est lui que tu mets dans le ThreadLocal.

Dans le tutoriel, on met dans le ThreadLocal un booléen. Il y a instance (le Singleton) et initHolder (le ThreadLocal contenant le booleen). Ce booléen est un indicateur pour savoir si le thread courant est déjà passé dans l'initialisation et donc ne pas faire la synchronisation et le test si instance est null.
C'est donc "interessant" si ton thread demande plusieurs fois le Singleton. Dans le cas de plusieurs threads de courte durée ne demandant qu'une fois le Singleton, ce n'est pas interessant. J'ai mis intéressant entre guillemet car le cout du ThreadLocal n'est pas forcément plus interessant que celui de la synchronisation du Singleton...
11/09/2006, 09h33
thibaut

Merci exact. C'est tout à fait clair. Mais j'ai été induis en erreur par un autre exemple du même acabis.

Merci