Discussion :

[thibaut]

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 :

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
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+

[BugFactory]

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.

[thibaut]

cf. http://christophej.developpez.com/tu...tithread/#L3.3

[divxdede]

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

[had35]

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

Code :

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

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 :

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
 
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;
  }
}

[had35]

La synchronisation de mes threads n'est pas bonne car, évidemment, il faudrait plutôt quelque chose comme ça :

Code :

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

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

[®om]

Pourquoi tu utilises ThreadLocal?

Pourquoi ne pas faire simplement:

Code :

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

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;
    }
 
}

[thibaut]

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

[had35]

Essayes un truc comme ça :

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
 
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();
		}
	}
}

[le y@m's]

Pourquoi tu utilises ThreadLocal?

Pourquoi ne pas faire simplement:

Code :

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

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 :

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

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;
    }

}

[thibaut]

Non c'est faux, car quand tu fais :

Code :

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

private static final BeanPool instance = new BeanPool();

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

Testé et approuvé.