views:

259

answers:

3

I have this piece of code that is giving me trouble. I know all the threads are reading the same struct. But I have no idea how to fix this.

#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>

typedef struct {
  int a,b;
} s_param;

void *
threadfunc(void *parm)
{
  s_param *param2 = parm; 
  printf("ID:%d and v:%d\n",param2->a,param2->b);
  pthread_exit(NULL);
}

int main(int argc, char **argv)
{
  pthread_t thread[3];
  int rc=0,i;
  void * status;

  for(i=0; i<3 ; ++i){
    s_param param;
    param.b=10;
    param.a=i;
    rc = pthread_create(&thread[i], NULL, threadfunc, &param ); // !!!!
    if(rc){
      exit(1);
    }
  }  

  for(i=0; i<3 ; ++i){
    pthread_join(thread[i],&status);
  }
  return 0;
}

output:

ID:2 and v:10
ID:2 and v:10
ID:2 and v:10

and what I need:

ID:0 and v:10
ID:1 and v:10
ID:2 and v:10
+4  A: 

The scope of the sparam in the for loop is static within that loop. When you set .a and .b you are writing to the same struct over and over, and all three threads are getting a pointer to that same single struct.

Instead, you could create three separate instances of the struct by making an array of them like so..

int main(int argc, char **argv)
{
  pthread_t thread[3];
  s_param param[3];
  int rc=0,i;
  void * status;

  for(i=0; i<3 ; ++i){
    param[i].b=10;
    param[i].a=i;
    rc = pthread_create(&thread[i], NULL, threadfunc, &param[i] ); // !!!!
    if(rc){
      exit(1);
    }
  } 
... etc

Should mention that creating the structs (and the threads) in an array like this is only feasible since you clearly do a join() with the main thread. If you didn't do that join, you would be advised to either statically allocate the structs outside of the main function, or malloc them from the heap, because as soon as the entry thread exits the main function, the stack frame containing the array will become invalid and will soon be overwritten in an unpredictable way.

JustJeff
+2  A: 

param lives on the same place in the stack during the execution of your main function. Every time you set new values, they're wiping out the old values, and all the threadfuncs are looking at the same spot in memory. malloc the structs, or otherwise create them from different memory locations. (Also, using stack memory for cross-thread data structures is worrisome; as soon as the function you're setting them up in exits, the memory is invalid.)

quixoto
+2  A: 

param is a local variable. It goes out of scope at the end of the loop's braces. You need to malloc a new s_param with each thread.

Or better, define a struct containing both a pthread_t and an s_param and use that type for thread[3].

typedef struct {
  int a,b;
  pthread_t t;
} s_param;

int main(int argc, char **argv)
{
  s_param thread[3]; // declare threads + params together
  int rc=0,i;
  void * status;

  for(i=0; i<3 ; ++i){
    s_param *param = &thread[i]; // no persistent data declared here
    param->b=10;
    param->a=i;
    rc = pthread_create(&thread[i].t, NULL, threadfunc, &thread[i] ); // !!!!
    if(rc){
      exit(1);
    }
  }  
…
Potatoswatter
`s_param *param = thread[i];` should be `s_param *param = `
Matthew Flaschen