Ecrasement de variable [Résolu]

[xc78370]

Bonsoir,

Dans une boucle, je crée plusieurs Threads pointant sur une même fonction. Or une des variable de cette fonction est écrasée lors du 2eme appel de cette fonction. Je me retrouve avec 2 fois le même résultat au lieu de 2 différents.

J'ai mis en place une section critique comme je l'ai lu à plusieurs endroits, mais rien n'y fait.

Y aurait-il quelque chose que je n'aurais pas compris ?

Voila mon code :

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
 
long C_TrtExec::Executer(void)
{
long i;
struct S_ThreadParam sO_ThreadParam;
HANDLE lO_ThreadId;
 
 
	for (i=0 ; i<cI_NbTrt ; i++) {
 
		sO_ThreadParam.sO_TrtExec = this;
		sO_ThreadParam.sO_Trt = cO_ListeTrt[i]; // *** Info envoyée au thread qui est différente ici à chaque appel mais qui ne l'est plus dans le thread
 
		lO_ThreadId = CreateThread(NULL, 0, ThreadLauncher, &sO_ThreadParam, 0, NULL);
 
		AddThread(lO_ThreadId)// Ajoute simplement le ThreadId dans une liste pour le WaitForMultipleObjects
 
	}
 
	WaitForMultipleObjects( cI_NbThread, cO_ListeThread, TRUE, INFINITE);
 
	DelAllThread();
 
	return 0;
}

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
 
DWORD WINAPI ThreadLauncher(void *pO_ThreadParam)
{
	EnterCriticalSection(&cO_CriticalSection); // cO_CriticalSection déclaré global
 
	struct S_ThreadParam *lO_ThreadParam = reinterpret_cast<struct S_ThreadParam *>(pO_ThreadParam);
	C_TrtExec *lO_TrtExec = lO_ThreadParam->sO_TrtExec;                          
	lO_TrtExec->TrtThread(lO_ThreadParam->sO_Trt); // Au deuxième appel, sO_Trt vaut la meme chose qu'au premier appel
 
	LeaveCriticalSection(&cO_CriticalSection);
 
	return 0;   
 
}

Merci de voter aide...

[Ehonn]

Le code est étonnamment compliqué pour ce qu'il est censé faire (?)
En C++11, il existe std::thread. http://en.cppreference.com/w/cpp/thread/thread
En C++ sinon, il y a boost::thread.
OpenMP peut être aussi intéressant.

As-tu fait un affichage avant la modification par les threads de lO_ThreadParam->sO_Trt (pour vérifier que la valeur est bien modifiée) ?
As-tu fait un affichage juste après être entré et juste avant de sortir de la section critique (en augmentant la durée du calcul à l'intérieur avec un sleep(10)) pour être sûr qu'elle soit respectée ?

[xc78370]

Envoyé par Ehonn

Le code est étonnamment pour ce qu'il est censé faire (?)

Qu'entends-tu par étonnamment long ?

Envoyé par Ehonn

En C++11, il existe std::thread. http://en.cppreference.com/w/cpp/thread/thread
En C++ sinon, il y a boost::thread.
OpenMP peut être aussi intéressant.

Ok, je vais jeter un coup d'oeil à ca !!!

Envoyé par Ehonn

As-tu fait un affichage avant la modification par les threads de lO_ThreadParam->sO_Trt (pour vérifier que la valeur est bien modifiée) ?

Oui, j'ai une log (j'ai enlevé les appels dans ce post), j'ai affiché l'objet avant de le passer au Thread ainsi que dans la fonction lO_TrtExec->TrtThread

Envoyé par Ehonn

As-tu fait un affichage juste après être entré et juste avant de sortir de la section critique (en augmentant la durée du calcul à l'intérieur avec un sleep(10)) pour être sûr qu'elle soit respectée ?

Non... je m'y mets...

[Iradrille]

Envoyé par xc78370

Qu'entends-tu par étonnamment long ?

C'est pas vraiment la longueur le soucis, c'est plutôt le fait de faire un code non portable alors que le C++ fourni tout ce qu'il faut pour faire ça simplement, de manière portable (std::thread depuis C++11, boost::thread avant)

Sinon pour ton problème, tu n'envoi que l'adresse de la structure qui contient tes params à tes threads.

Tu n'as donc q'un seul objet ThreadParam existant, et tous les threads ont forcément les mêmes paramètres.

[Ehonn]

Envoyé par Ehonn

Le code est étonnamment pour ce qu'il est censé faire (?)

étonnamment compliqué* (j'ai oublié un mot).

[xc78370]

Envoyé par Iradrille

C'est pas vraiment la longueur le soucis, c'est plutôt le fait de faire un code non portable alors que le C++ fourni tout ce qu'il faut pour faire ça simplement, de manière portable (std::thread depuis C++11, boost::thread avant)

OK, j'avoue que je ne fais pas trop gaffe à ça, développant tout le temps pour la meme plateforme... Je vais y faire attention.

Envoyé par Iradrille

Sinon pour ton problème, tu n'envoi que l'adresse de la structure qui contient tes params à tes threads.

Tu n'as donc q'un seul objet ThreadParam existant, et tous les threads ont forcément les mêmes paramètres.

On ne peut pas envoyer autre chose qu'un pointeur à Createthread...

[xc78370]

Envoyé par Ehonn

étonnamment compliqué* (j'ai oublié un mot).

??? C'est à dire ?

[Iradrille]

Envoyé par xc78370

On ne peut pas envoyer autre chose qu'un pointeur à Createthread...

Utilise un tableau (si cI_NbTrt est une constante de compilation), un vector sinon.

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
long C_TrtExec::Executer(void)
{
	long i;
	struct S_ThreadParam sO_ThreadParam[cI_NbTrt];
	HANDLE lO_ThreadId;
 
 
	for (i=0 ; i<cI_NbTrt ; i++) {		
		sO_ThreadParam[i].sO_TrtExec = this;
		sO_ThreadParam[i].sO_Trt = cO_ListeTrt[i]; // *** Info envoyée au thread qui est différente ici à chaque appel mais qui ne l'est plus dans le thread
 
		lO_ThreadId = CreateThread(NULL, 0, ThreadLauncher, &sO_ThreadParam[i], 0, NULL);
 
		AddThread(lO_ThreadId)// Ajoute simplement le ThreadId dans une liste pour le WaitForMultipleObjects
 
	}
 
	WaitForMultipleObjects( cI_NbThread, cO_ListeThread, TRUE, INFINITE);
 
	DelAllThread();
 
	return 0;
}

[Arzar]

Vu qu'un thread met un peu de temps avant de se lancer il est très probable que la boucle for soit déjà terminée lorsque les ci_Nbtr threads démarrent pour de bon en appelant la fonction ThreadLauncher, et comme ils prennent tous en paramètre un pointeur vers la même structure sO_ThreadParam, ils partagent tous le même champ sO_Trt.

Il faut remplacer cette structure par un tableau et donner au ième thread l'adresse de la ième structure

Code :

S_ThreadParam sO_ThreadParam[NbTrt ];

Autre chose, je ne sais pas trop quel est le but recherché avec tous ces threads mais dans le code montré ils ne servent à rien du tout. Comme la fonction ThreadLauncher est protégé par un mutex, tous les thread vont attendre un par un que le mutex se libère et la fonction TrtThread () va être appelé à tour de rôle, exactement comme si on avait écrit :

Code :

1
2
3
4
for (i=0 ; i< cI_NbTrt ; i++) 
{
    TrtThread(cO_ListeTrt[i]);
}

[xc78370]

Envoyé par Arzar

Vu qu'un thread met un peu de temps avant de se lancer il est très probable que la boucle for soit déjà terminée lorsque les ci_Nbtr threads démarrent pour de bon en appelant la fonction ThreadLauncher, et comme ils prennent tous en paramètre un pointeur vers la même structure sO_ThreadParam, ils partagent tous le même champ sO_Trt.

Il faut remplacer cette structure par un tableau et donner au ième thread l'adresse de la ième structure

Code :

S_ThreadParam sO_ThreadParam[NbTrt ];

Autre chose, je ne sais pas trop quel est le but recherché avec tous ces threads mais dans le code montré ils ne servent à rien du tout. Comme la fonction ThreadLauncher est protégé par un mutex, tous les thread vont attendre un par un que le mutex se libère et la fonction TrtThread () va être appelé à tour de rôle, exactement comme si on avait écrit :

Code :

Merci beaucoup. J'ai effectivement suivi le conseil d'Iradrille (qui est le même que le tient Arzar) et cela fonctionne ...
Au final la section critique n'apportait rien, je l'ai donc enlevée.
Concernant la portabilité de mon code, je vais m'attacher à faire attention.

Merci à tous.

[Iradrille]

Envoyé par xc78370

Au final la section critique n'apportait rien, je l'ai donc enlevée.
Concernant la portabilité de mon code, je vais m'attacher à faire attention.

A titre d'exemple, le même code en utilisant des std::thread (pas testé pas de compilo suportant C++11 sous la main )

Code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
long C_TrtExec::Executer() {
	std::vector<std::thread> threads;
 
	for (long i=0; i<cI_NbTrt; ++i) {	
		S_ThreadParam sO_ThreadParam;	
		sO_ThreadParam.sO_TrtExec = this;
		sO_ThreadParam.sO_Trt = cO_ListeTrt[i];
 
		// passage du paramètre par copie
		threads.push_back(std::thread(ThreadLauncher, sO_ThreadParam));
	}
 
	for(auto t: threads) {
		t.join();
	}
 
	return 0;
}
 
void ThreadLauncher(pO_ThreadParam) {
	pO_ThreadParam.sO_TrtExec(pO_ThreadParam.sO_Trt);
}

[xc78370]

Envoyé par Iradrille

A titre d'exemple, le même code en utilisant des std::thread (pas testé pas de compilo suportant C++11 sous la main )

Merci pour le code, mais je suis comme toi, je travaille en VCPP6 ()

[Ehonn]

Voici une utilisation de std::thread et std::mutex
Avec une fonction et une lamda_function (juste pour l'exemple)

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
// g++ -Wall -Wextra -std=c++11 -pedantic thread_mutex.cpp -o thread_mutex && ./thread_mutex
 
#include <iostream>
#include <chrono>
#include <thread>
#include <mutex>
 
 
// Fonction pour le thread
void fct(int & v, std::mutex & m)
{
	std::cout << "Avant mutex\n";
	m.lock();
 
	std::cout << "Début mutex\n";
 
	// On dort 10 secondes
	std::this_thread::sleep_for(std::chrono::seconds(10));
 
	// On incrémente la valeur
	++v;
 
	std::cout << "Fin mutex\n";
	m.unlock();
}
 
 
int main()
{
	// On peut aussi utiliser une lambda fonction
	auto lambda_fct = [](int & v, std::mutex & m) -> void
	{
		std::cout << "Avant mutex\n";
		m.lock();
 
		std::cout << "Début mutex\n";
 
		// On dort 10 secondes
		std::this_thread::sleep_for(std::chrono::seconds(10));
 
		// On incrémente la valeur
		++v;
 
		std::cout << "Fin mutex\n";
		m.unlock();
	};
 
	int v = 0;
	std::mutex m;
 
	std::thread t0(lambda_fct, std::ref(v), std::ref(m));
	std::thread t1(fct,        std::ref(v), std::ref(m));
 
	t0.join();
	t1.join();
 
	std::cout << "Valeur finale de v = " << v << std::endl;
 
	return 0;
}

[xc78370]

Ok merci pour le code.

Je garde ca sous le coude pour des jours meilleurs !!!!