Thread : accumulation et fuites mémoires

Version imprimable

Bonjour,

A priori, vu les fuites mémoires et le nombre de threads qui augmente sans cesse, il doit y avoir un problème dans la façon dont c'est codé dans l'application.

Voici la fonction appelée régulièrement :
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 26 27 28 29 30 @Transactional public List<InformationValideur> lectureJournee() { try { //Toutes les 60s : nouveau thread de rechargement @Transactional final class Launch implements Runnable { public void run() { try { if (!ChargementJourneeEnCours) { ChargementJourneeEnCours = true; listeInformationValideurJournee = lectureJourneeCourante(); dateDernierChargementJournee = new Date(); ChargementJourneeEnCours = false; } } catch (Exception e) { e.printStackTrace(); ChargementJourneeEnCours = false; dateDernierChargementJournee = null; } } } Launch l = new Launch(); Thread internalThread = new Thread(l); internalThread.start(); } catch (Exception e) { ChargementJourneeEnCours = false; } return listeInformationValideurJournee; }
-->est-ce que quelque chose de mal saute aux yeux?
1°/Devrait-on explicitement glisser dans la méthode run() des instructions Thread.currentThread().interrupt(); ?
2°/Normalement on s'assure de n'appeler lectureJourneeCourante() qu'une fois mais est-ce que mettre cette fonction en synchronized peut être utile?

Merci.
Ci-dessous le code plus complet.

Ici, je me demande si je c'est correct cette façon d'utiliser ma (petite) liste temporaire "lstModifByAction" pour mettre à jour ma (grande) liste principale "res".
Est-ce que mon new ArrayList<InformationValideur>() fait effectivement ce que je pense faire ou une histoire de pointeur fait que ce passage expliquerait mes fuites mémoires?

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
26
27
28
29
	@Transactional
	public List<InformationValideur> lectureJourneeCourante() {
		List<InformationValideur> res = new ArrayList<InformationValideur>();
 
		//Le but est de recharger toute la liste des lignes du tableau
		//1°/Chargement de toutes les données du jour
 
		//Inutile de recopier tout le code ici
 
		//2°/Modification des éléments nécessaires
		//On a constitué à chaque action utilisateur une liste lstModifByAction
		//On s'en sert pour aller modifier la liste globale 
		for (InformationValideur pModif : lstModifByAction) {
				for (int i = 0; i < res.size(); i++) {
					pToReturn = res.get(i);
					if (pToReturn.getIdentifiant().equals(pModif.getIdentifiant())) {
						pToReturn = pModif;
						res.set(i, pToReturn);
						break;
					}
				}
			}
 
		//et quand on a fini on vide la liste
		lstModifByAction = new ArrayList<InformationValideur>();
 
		return res;
 
	}

Et enfin voici l'ensemble de tout ce qui concerne ce processus :

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
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
/********************************************************************************************************/
/****************************** AppListener.java****************************************************/
/********************************************************************************************************/
 
public void contextInitialized(ServletContextEvent sce) {
		super.contextInitialized(sce);
		ClassLoaderControler.getInstance().load();
		com.his.common.CommonContext.registerContext(com.his.App.AppContext.class, sce.getServletContext());
		AppContext.getInstance().demarrerTimerRecuperation();
	}
 
 
/********************************************************************************************************/
/****************************** AppContext.java ****************************************************/
/********************************************************************************************************/
 
	public void demarrerTimerRecuperation() {
		Timer timer = new Timer();
		/* schedule des données toutes les 120 secondes */
		timer.schedule(new LectureJournee(), new Date(), 120 * 1000); /*120 secondes*/
	}
 
	class LectureJournee extends TimerTask {
		public void run() {
			Calendar cal = Calendar.getInstance();
			int h = cal.get(Calendar.HOUR_OF_DAY);
			if (h >= AppConstants.CACHE_HEURE_DEBUT && h <= AppConstants.CACHE_HEURE_FIN) {
				AppContext.getInstance().getInformationService().lectureJournee();
			} else {
				log.info(new Date() + " rechargement...sleeping time");
			}
		}
	}
 
 
/********************************************************************************************************/
/****************************** InformationService.java *************************************************/
/********************************************************************************************************/
 
@Transactional
public List<InformationValideur> lectureJournee() {
	try {
		//Toutes les 60s : nouveau thread de rechargement
		@Transactional
		final class Launch implements Runnable {
			public void run() {
				try {
					if (!ChargementJourneeEnCours) {
						ChargementJourneeEnCours = true;
						listeInformationValideurJournee = lectureJourneeCourante();
						dateDernierChargementJournee = new Date();
						ChargementJourneeEnCours = false;
					}
				} catch (Exception e) {
					e.printStackTrace();
					ChargementJourneeEnCours = false;
					dateDernierChargementJournee = null;
				}
			}
		}
 
		Launch l = new Launch();
		Thread internalThread = new Thread(l);
		internalThread.start();
		} catch (Exception e) {
			ChargementJourneeEnCours = false;
		}
		return listeInformationValideurJournee;
	}
 
/********************************************************************************************************/
/****************************** InformationService.java ***********************************************/
/********************************************************************************************************/
 
	@Transactional
	public List<InformationValideur> lectureJourneeCourante() {
		List<InformationValideur> res = new ArrayList<InformationValideur>();
 
		//Le but est de recharger toute la liste des lignes du tableau
		//1°/Chargement de toutes les données du jour
 
		//Inutile de recopier tout le code ici
 
		//2°/Modification des éléments nécessaires
		//On a constitué à chaque action utilisateur une liste lstModifByAction
		//On s'en sert pour aller modifier la liste globale 
		for (InformationValideur pModif : lstModifByAction) {
				for (int i = 0; i < res.size(); i++) {
					pToReturn = res.get(i);
					if (pToReturn.getIdentifiant().equals(pModif.getIdentifiant())) {
						pToReturn = pModif;
						res.set(i, pToReturn);
						break;
					}
				}
			}
 
		//et quand on a fini on vide la liste
		lstModifByAction = new ArrayList<InformationValideur>();
 
		return res;
 
	}

16/07/2013, 14h28
olivier.pitton

Plop,

Ce que je ne comprends pas dans ton code, c'est pourquoi tu utilises un thread pour les lectures ? Tu as lancé ton timer toutes les 120 sec donc ce code est déjà mis dans un thread à part. Tu devrais enlever toute la partie thread de ton code et mettre le code business directement dans le TimerTask.

Le fait que ton timer soit un autre thread te permet déjà d'effectuer le traitement en parallèle et t'éviteras des effets de bord. Par exemple, lorsque tu lances ton thread, tu renvoie directement la liste modifiée après son lancement, or le thread créé fait des choses. Donc si le code du thread est exécuté avant le return, tu es bon, sinon tu renvoies la précédente liste (puisque le thread n'a pas encore rafraichit tes données).

Citation:

1°/Devrait-on explicitement glisser dans la méthode run() des instructions Thread.currentThread().interrupt(); ?

En utilisant un débugger, regarde si le nombre de threads croit sans arrêt. Si oui, il faudra penser à les tuer (ou simplement ne pas les créer comme dit juste avant). Mais évite de faire cela quand tu le peux. Je pense que ton principal problème vient de la création de threads inutiles.

Citation:

2°/Normalement on s'assure de n'appeler lectureJourneeCourante() qu'une fois mais est-ce que mettre cette fonction en synchronized peut être utile?

Non. Tu crées un thread à la fois donc tu n'as qu'un seul thread dans cette méthode. Si cette méthode met beaucoup de temps à s'exécuter, alors tu vas avoir au bout d'un moment une espèce de file d'attente (toutes les 2 min, un thread est exécuté pendant 30 sec, donc au bout d'1min 30 tu vas créer le 2ème etc...). Ce n'est pas le problème pour moi.

Citation:

Est-ce que mon new ArrayList<InformationValideur>() fait effectivement ce que je pense faire ou une histoire de pointeur fait que ce passage expliquerait mes fuites mémoires?

Ici, tu veux juste vider la liste si j'ai bien compris. Pourquoi ne pas utiliser la méthode clear() au lieu de créer une nouvelle liste. Quoiqu'il en soit, a moins que ta liste soit référencée ailleurs, je ne vois pas comment elle ne pourrait pas être nettoyée.

Tu parles de fuites mémoires mais qu'est ce qui fuit ? Quelles sont les types de données qui reste et dans où ? Essaye d'utiliser un profiler pour nous dire cela.
16/07/2013, 15h00
tchize_

Même si c'est une méthode pas très propre pour le faire, on va supposer donc que tu appelle comme tu le dit "lectureJournée" toutes les 60 secondes, et on va supposer que ça met moins de 60 secondes pour se terminer.

La question qui se pose, c'est qu'est-ce que tu fais de listeInformationValideurJournee quand d'autre méthodes récupèrent cette valeur. En effet, comme toutes les 60 secondes tu remplace cette liste par une autre, si l'appelant conserve l'ancienne liste, plus tu aura eu d'appel, plus tu aura eu d'ancienne liste conservées.

Aussi, pourquoi un thread à part dans lectureJournee? Puisque cette methode est appelée toutes les 60 secondes, tu peux directement dedans faire le chargement des données, pourquoi faire ça en asynchrone comme tu le fais?

Merci. Je réfléchis en détail à vos réponses demain.

Juste pourquoi mon prédécesseur a écrit un timer + un thread je ne sais pas exactement mis à part que dans le code de lectureJournee() lancé pour rappel toutes les 2 minutes :
-si ça fait moins de 30s que le dernier rechargement s'est fini, on prend dans le cache
sinon :
-si ça fait moins de 60s que le dernier rechargement s'est fini, on prend dans le cache et on lance un thread de rechargement
sinon :
-on recharge les données
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 if (dateDernierChargementJournee != null && dateDernierChargementJournee.after(cal.getTime())) { // Moins de 30s on reutilise le cache log.info(new Date() + " moins de 30s on reutilise le cache"); } else { cal.setTime(curDate); cal.add(Calendar.SECOND, -60); if (dateDernierChargementJournee != null && dateDernierChargementJournee.after(cal.getTime())) { // Moins de 60s on reutilise le cache et on relance le chargement Launch l = new Launch(); Thread internalThread = new Thread(l); internalThread.start(); } else { if (!ChargementJourneeEnCours) { ChargementJourneeEnCours = true; listePrevalidationGerantJournee = lectureJourneeCourante(); dateDernierChargementJournee = new Date(); ChargementJourneeEnCours = false; } } }

17/07/2013, 09h15
tchize_

Je dois être myope, je ne vois nulle part de test relatif aux 30 secondes. Ce que je vois c'est

une timertask qui tourne toutes les 2 minutes
qui teste si on est dans la tranche [DEBUT,FIN]
et si oui, lance une méthode qui:
systématiquement crée un thread de rechargement de données
renvoie à l'appelant les données en cache.

La question est

1) pourquoi faire un thread à part, on peux très bien laisser le timertask attendre la fin du rechargement, puisqu'elle ne fait rien d'autre
1.bis) En plus, ça veux dire que le chargement se fait en dehors de la transaction du coup (remarque, c'est peut-être le but recherché, mais dans ce cas faut le documenter)
2) si la raison est qu'on appelle lectureJournee de plein d'autres endroits dans le code, alors il faut bien être conscient que ça veux dire que ton application va passer son temps à lancer des threads à tout va pour recharger le cache.
17/07/2013, 14h05
stof
Citation:

Envoyé par tchize_

Je dois être myope, je ne vois nulle part de test relatif aux 30 secondes.

J'ai pas affiché le morceau de code qui précdède qui fait :
Code:

1 2 3 4 Date curDate = new Date(); Calendar cal = Calendar.getInstance(); cal.setTime(curDate); cal.add(Calendar.SECOND, -30);
Voilà ce que fait l'application :
On a un tableau avec une liste d'informations
A]Toutes les 2mn, ça se rafraichit tout seul
B]Sur certaines actions d'un utilisateur, ça modifie aussitôt les informations de la ligne modifiée (visuellement tant que le passage du process de rechargement n'a pas rechargé toute la liste)

Quand on lance une recherche sur certains critères, on n'appelle pas lectureJournee mais on "tape" dans la liste qui est régulièrement mise à jour.

Donc, comme on n'est pas spécialement pressé d'avoir la liste, le passage qui dit qu'entre 30s et 60s on lance un thread n'est probablement pas utile.
-->moins de 30s, on ne recharge pas
-->sinon, à confition de ne pas être en cours de rechargement, on fait tous nos traitements de rechargement et on renvoie la liste mise à jour.
17/07/2013, 14h28
tchize_

sauf que le code que tu montre ne renvoie pas la liste à jour mais l'ancienne liste, après avoir demandé à un thread à part de recharger "quand il pourra".
17/07/2013, 15h18
stof

Citation:

Envoyé par tchize_

sauf que le code que tu montre ne renvoie pas la liste à jour mais l'ancienne liste, après avoir demandé à un thread à part de recharger "quand il pourra".

Oui, comme je l'ai présenté ci-dessus, c'était bien l'"ancienne" liste qui était renvoyée (pas grave, elle est pas si vieille que ça) avec demande de rechargement parallèle.

Mais dans mon dernier post, j'évoquais plutôt le principe que j'allais mettre en place. Je pense me passer du thread oui (bon, après je ne sais pas si ça va révolutionner mon application mais autant simplifier quand on peut).
17/07/2013, 16h18
tchize_

Ceci dit, a moins que tu n'aie un erreur qui lance 200 fois le thread par seconde, ce n'est pas la cause de ta fuite mémoire.

Le meilleur moyen de trouver une fuite mémoire, ça reste le profiling de l'application ;)