Discussion :

[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 :

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

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

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]

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

Qu'entends-tu par étonnamment long ?

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

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

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]

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]

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

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

[xc78370]

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.

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]

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

??? C'est à dire ?

[Iradrille]

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 :

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
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électionner tout - Visualiser dans une fenêtre à part

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 :

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

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

[xc78370]

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électionner tout - Visualiser dans une fenêtre à part

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 :

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

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

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]

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 :

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

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 :

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