Discussion :

[kodo]

Bonjour,
J'ai une classe singleton qui contient la méthode suivante, qui fait un simple Select :

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
26
27
28
29
30
31
32
33
34
35
36
public class UtilisateursManagerImpl implements UtilisateursManager {
	private EntityManagerFactory emf;
	private EntityManager entityManager;
 
	private static final Logger logger = LogManager.getLogger(UtilisateursManagerImpl.class.getSimpleName());
 
	public Object initialize() {
		if(emf!=null)
			entityManager = emf.createEntityManager();
		else
			logger.error("ERROR : emf NULL");
		return entityManager;
	}
 
	public synchronized boolean isValidUser(String login) throws Exception {
		List listResult;
		UserBean user = null;
		try {
			StringBuilder nativeSQL = new StringBuilder("");
			nativeSQL.append("select user.ID_UTILISATEUR from UTILISATEUR user where user.LOGIN_UTILISTEUR = :loginUser and user.EST_ACTIF = 1 ");
			Query queryNative = entityManager.createNativeQuery(nativeSQL.toString());
			queryNative.setParameter("loginUser", login);
			listResult = queryNative.getResultList();
			if (listResult != null && listResult.size() != 0)
				return true;
			return false;
		} catch (Exception de) {
			logger.error(de.getMessage());
			de.printStackTrace();
			entityManager.clear();
			throw new Exception(de.getMessage());
		} finally {
			entityManager.clear();
		}
	}
}

Ma question est ce que le fait de la rendre synchronisé est justifiable ?
Pour info, c'est spring qui gère la création de factory EMF et l'injection.

Merci.

[OButterlin]

Je ne vois pas d'information concernant le côté "Singleton" dans ce code et je ne sais pas comment Spring gère un Singleton.

Cependant, me basant sur les EJB, Singleton veut dire qu'il y a une instance unique dans la JVM, ça ne veut pas dire que la méthode ne peut pas être appelée de plusieurs endroits (plusieurs thread).

Du coup, ça pourrait être justifié de mettre la méthode en syncrhonized dans une classe Singleton (Avec les EJB, on a l'annotation @LOCK en plus qui permet de verrouiller).

Ceci dit, dans ce cas, je ne vois pas trop l'intérêt vu que la méthode ne fait qu'une lecture pour déterminer si l'utilisateur est valide et actif.

Donc, en résumé, pour moi et dans ce cas précis --> pas besoin

[thelvin]

A la rigueur si l'EntityManager était incapable de gérer plus d'une connexion à la fois...

Genre s'il maintenant sa connexion à la BDD en lieu et place d'en demander une à chaque fois (en principe venant d'un pool de connexion)

Mais bon, s'il fait ça les accès concurrents sont pas le seul problème, c'est déjà fragile en soi.

[kodo]

Je ne vois pas d'information concernant le côté "Singleton" dans ce code et je ne sais pas comment Spring gère un Singleton.

je voulais dire que j'ai un fichier de configuration bean.xml qui contient la ligne suivante :

Code :

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

<bean id="UtilisateursManager" class="com.ips.business.managers.impls.administration.UtilisateursManagerImpl"></bean>

Donc, en résumé, pour moi et dans ce cas précis --> pas besoin

On pourrait pas avoir ce cas, deux threads qui exécutent ce code, un est en train d'exécuter le select, l'autre est dans le block finally qui exécute la méthode clear, et donc exception pour le premier ?

[OButterlin]

Dans une programmation multi-thread, chaque thread a sa pile.
En l'occurrence, la connexion peut être la même mais les "statements" rattachés sont distincts, du coup, il n'y a pas de risque.
Dans tous les cas, comme les piles d'exécutions ne sont pas les mêmes, tout plantage dans un thread n'a pas d'incidence dans le second, sauf à partager des éléments (Ici, si la connexion est partagée, une fermeture de la connexion dans le thread 1 aurait un impact sur le thread2).

[kodo]

Merci pour les retours.

Ici nous avons une bean singleton avec un objet entityManager
Qui dit singleton, dit un seul objet dans la JVM. Donc tous les threads utilisent le même objet pour communiquer avec la base de données. Ce qui change est la connexion obtenue depuis le pool de connexion à chaque select. Donc je pense que nous avons toujours ce risque en cas d'accès concurrent. Non ?

[OButterlin]

Non, chaque thread va créer l'instance Query dans sa pile, du coup, même si c'est la même méthode, ce ne sont pas les mêmes objets en mémoire.

Comme le soulignait très justement thelvin, à moins d'avoir plus de demandes simultanées que l'entityManager n'est capable de gérer (en fonction de la base de données et du driver, une connexion peut avoir plus ou moins de statements rattachés), il n'y a pas de risque.

[kodo]

Non, chaque thread va créer l'instance Query dans sa pile, du coup, même si c'est la même méthode, ce ne sont pas les mêmes objets en mémoire.

Mais l'instance d'entityManager qui est une variable de classe, on n'aura qu'une 1 dans le heap. donc c'est la seule instance utilisée par tous les thread

[OButterlin]

Tout à fait.

Mais il faut voir l'instance de l'EntityManager pour ce qu'elle est dans ce cas (NativeQuery) -> une connexion. (on est d'accord que l'EntityManager fait plus que ça mais ici, on peut le voir comme une simple connexion (Connection au sens JDBC)).
Or, une connexion peut avoir plusieurs instances de Statement attachées (ici Query), sans l'ombre d'un problème.... jusqu'au moment où on dépasse le nombre d'instances rattachables à la connexion (Statement, PreparedStatement, etc...).

Donc, pour résumé ce qui a déjà été dit, si tu penses que plus de thread qu'il n'y a d'instances rattachables à la connexion peuvent accéder en même temps à cette méthode, alors ajoute le synchronized qui te garantit qu'il n'y en aura qu'une seule à un instant T mais qui, en contrepartie, va pénaliser les performances.
Sinon, pas besoin du synchronized.
Et si tu ne sais pas répondre à cette question -> utilise synchronized

[OButterlin]

J'oubliais :

Pour moi, il y a une erreur d'architecture fondamentale dans ce code.
L'instance EntityManager devrait être membre de la méthode, pas de la classe.

Pour la classe, ok pour l'instance commune de la factory.
Pour l'EntityManager, c'est autre chose vu qu'il encapsule une connexion à la base de données.
Dans ton cas, ça veut dire qu'il va "verrouiller" une connexion pour toute la durée de vie de l'application...très mauvaise idée

Donc, si tu déplaçais l'acquisition de l'EntityManager dans ta méthode, non seulement tu ne verrouillerais pas la connexion mais en plus la question du synchronized ne se poserais plus... à toi de voir