[C][thread] problèmes valeur de retour pthread_join

[doul11]

Bonsoir,

J'essaye de comprendre le fonctionnement des threads, je me suis inspiré du programme d'exemple de la page man pthread_create pour écrire un petit code afin de tester différentes fonctions, j'ai un problème avec la valeur de retour (voir le code et la sortie du programme ci dessous), le retour du thread[0] n'est jamais bon alors que les suivants oui ! de plus si je demande 10 thread j'ai une erreur de segmentation, sinon j'ai un "abandon" ??? après le retour de thread[4] !

Votre aide serai la bien venue, merci.

#include<stdio.h>
#include<unistd.h>
#include<pthread.h>
#include <sys/syscall.h>


typedef struct
{
	pthread_t thread_id;
	int thread_num;
}ThreadInfo;


void *fx(void *arg)
{
ThreadInfo *thread_info = (ThreadInfo *) arg;
     
thread_info->thread_id=syscall(SYS_gettid);
printf("fx : new thread[%i] : %i\n",thread_info->thread_num, thread_info->thread_id);
     
sleep(thread_info->thread_num*2+5);

printf("fx : thread[%i] : end !\n",thread_info->thread_num);

char s[256];
sprintf(s,"thread[%i] sleep time : %i",thread_info->thread_num,thread_info->thread_num*2+5);
return &s;
}


int main()
{
int i,n;

printf("Numbers of threads ?\n");
if( (scanf("%i",&n))!=1){printf("scanf error ! \n");return -1;}
     
pthread_t *thread;
thread=malloc(sizeof(thread)*n);
     
ThreadInfo *thread_info;
thread_info=malloc(sizeof(thread_info)*n);

pthread_attr_t thread_attr;


for(i=0;i<n;i++)
	{
	thread_info[i].thread_id=0;
	thread_info[i].thread_num=i;
	
	if( (pthread_attr_init(&thread_attr))!=0 ){perror("create thread attributes error");return -1;}
	
	if( (pthread_create(&thread[i],&thread_attr,&fx,&thread_info[i]))!=0 ){perror("create thread error");return -1;}
     
	while(thread_info[i].thread_id==0);
	printf("main : create thread[%i] : %i\n",i,thread_info[i].thread_id);
     
	if( (pthread_attr_destroy(&thread_attr))!=0 ){perror("destroy thread attributes error");return -1;}
    	}


void *res;

for(i=0;i<n;i++)
	{
	res=NULL;
    	if( (pthread_join(thread[i], &res))!=0 ){perror("join thread error !\n");return -1;}
    	printf("main : joined with thread[%i] returned value : <%s>\n",i, (char *) res);
    	}

return 0;
}

sortie du programme :

[doul@localhost thread]$ ./thread 
Numbers of threads ?
5
main : create thread[0] : 5935
fx : new thread[0] : 5935
main : create thread[1] : 5936
fx : new thread[1] : 5936
main : create thread[2] : 5937
fx : new thread[2] : 5937
main : create thread[3] : 5938
fx : new thread[3] : 5938
main : create thread[4] : 5939
fx : new thread[4] : 5939
fx : thread[0] : end !
main : joined with thread[0] returned value : <��Z��>o�>
fx : thread[1] : end !
main : joined with thread[1] returned value : <thread[1] sleep time : 7>
fx : thread[2] : end !
main : joined with thread[2] returned value : <thread[2] sleep time : 9>
fx : thread[3] : end !
main : joined with thread[3] returned value : <thread[3] sleep time : 11>
fx : thread[4] : end !
Abandon
[doul@localhost thread]$

-----

[polo974]

char s[256];
sprintf(s,"thread[%i] sleep time : %i",thread_info->thread_num,thread_info->thread_num*2+5);
return &s;

Tu alloues sur la pile du thread une chaine et tu espères retrouver ce que tu y a mis alors que tu rends la main au niveau de la fonction ET au niveau du thread...

En suite, si le compilo n'a pas gueulé, c'est très bizare (return &s, s étant déjà le pointeur sur s[256])...

Toute données déclarées locales d'une fonction sont à considérer comme perdues une fois sorti de la fonction. Souvent ça marche, mais un jour ou l'autre ça te pète à la gueule telle une vieille mine oubliée et à moitié rouillée (donc très dure à déminer...)

En plus entre threads... C'est encore plus casse gueule. ajoute s dans ta structure, et ça ira mieux...

[doul11]

Bonsoir,

Je me doutais bien que c'était pas bon de renvoyer un pointeur sur une variable locale (gcc me l'avait dit) mais alors pourquoi c'est écrit comme cela dans la page man ?

Du coup j'ai viré pthread_join, j'ai mis s dans la structure et j'ai ajouter un flag qui indique si les thread est "running".

Sinon j'ai trouvé le "vrais" problème :

ThreadInfo *thread_info;
thread_info=malloc(sizeof(thread_info)*n);
...
thread_info[i].thread_id=0;

ça c'est pas bon, c'est soit un tableau de structures:

ThreadInfo *thread_info;
thread_info=malloc(sizeof(*thread_info)*n);
...
thread_info[i].thread_id=0;

soir un tableau de pointeurs sur une structure

ThreadInfo **thread_info;
thread_info=malloc(*sizeof(thread_info)*n);
...
thread_info[i]=malloc(**thread_info);
thread_info[i]->thread_id=0;

Au fait quelle est la meilleure solution : tableau de structures ou tableau de pointeurs sur une structure ?

[doul11]

oups ! j'ai écrit trop vite hier soir ...

ThreadInfo **thread_info;
thread_info=malloc(*sizeof(thread_info)*n);
...
thread_info[i]=malloc(**thread_info);
thread_info[i]->thread_id=0;

correction :

ThreadInfo **thread_info;
thread_info=malloc(sizeof(*thread_info)*n);
...
thread_info[i]=malloc(sizeof(**thread_info));
thread_info[i]->thread_id=0;

[A voir en vidéo sur Futura]

[polo974]

euuuuh, là tu te compliques la vie...
A moment où tu a n, tu connais la taille de ton tableau,
donc tu déclares un pointeur sur la struct:

 ThreadInfo *thread_info;

tu fais le malloc du tableau:

 thread_info=malloc(sizeof(thread_info)*n);

maintenant, tu as un pointeur sur une structure.
une zone mémoire pouvant contenir tes structures
tu n'a plus qu'à y accéder (thread_info[i] est une structure, on utilise le point:

 thread_info[i].thread_id=0;

mais si tu tiens à utiliser les '->' plutôt que le '.':

 (thread_info+i)->thread_id=0;

ce qui est plus long et moins lisible...

et quand tu n'en a plus besoin, un petit:

free(ThreadInfo)

Enfin, pourquoi réinventer le pthread_join avec un flag que tu vas lire dans une boucle en bouffant 100% du cpu... (toujours faire gaffe à ça: pas de boucle d'attente sans un minimum de tempo (minimum à définir...)).

[doul11]

tu fais le malloc du tableau:

 thread_info=malloc(sizeof(thread_info)*n);

maintenant, tu as un pointeur sur une structure.
une zone mémoire pouvant contenir tes structures

t'est sur ?

je suis sur que c'est pas ça, quand tu fait ça tu a un pointeur sur une zone mémoire de n pointeurs (thread_info étant un pointeur sur une structure ThreadInfo), pas de n structures, pour ça il faut faire malloc(sizeof(*thread_info)*n) ; (*thread_info étant l'objet pointé par thread_info) qui est équivalent a malloc(sizeof(ThreadInfo)*n); sur une petite zone ça marche quand même, mais sur un gros volume c'est l'erreur de segmentation assurée ! (j'ai essayé)

typedef struct
{
	double x;
	double y;
}Point;


int main()
{

Point *point;

printf("sizeof point : %i\n",sizeof(point));
printf("sizeof double : %i\n",sizeof(double));
printf("sizeof *point : %i\n",sizeof(*point));
printf("sizeof Point : %i\n",sizeof(Point));
return 0;
}

ça donne en sortie :

[doul@localhost malloc]$ ./malloc 
sizeof point : 4
sizeof double : 8
sizeof *point : 16
sizeof Point : 16

Enfin, pourquoi réinventer le pthread_join avec un flag que tu vas lire dans une boucle en bouffant 100% du cpu... (toujours faire gaffe à ça: pas de boucle d'attente sans un minimum de tempo (minimum à définir...)).

merci pour ce bon conseil !

[polo974]

t'est sur ?
...

Oups, je pensais :

  thread_info=malloc(sizeof(ThreadInfo)*n);

Quelle idée de donner des noms de variable trop ressemblants au nom de la structure ???
(il faut bien que je me trouve une excuse...)

ensuite, ça roule...