C Multithreading | Thread creation in for loop uses args from last iteration

Question

I'm new to multithreading and not the best in C in general so bare with me.

I have a for loop that creates a number of threads to which I'm passing arguments as such:

  for(int i = 0; i < NO_OF_THREADS; i++) {

    int ordered_product = (rand() % NO_OF_PRODUCTS);
    int ordered_quantity = (rand() % 10) + 1;
    int customer = (rand() % NO_OF_CUSTOMERS);

    printf("%d %d %d\n", customer+1, ordered_quantity, ordered_product+1);

    ThreadArgs myargs = {customer, ordered_product, ordered_quantity};

    int rc = pthread_create(&mythreads[i], NULL, thread_function, &myargs);
    if(rc != 0) {
      perror("Pthread create");
      exit(1);
    }

  }

and I have the function "thread_function" which is written as such:

void* thread_function(void* arg) {

  ThreadArgs* args = (ThreadArgs*) arg;
  ThreadArgs myargs = *args;
  int customer_id = myargs.customer_id + 1;
  int product_quantity = myargs.product_quantity;
  int product_id = myargs.product_id +1;

  printf("Customer %d purchased %d of Product %d\n", customer_id, product_quantity, product_id);

  //pthread_exit(NULL)   // I tried this too...
  return NULL;
}

This is the output that I'm getting:

4 8 4
3 3 9
8 1 9
Customer 8 purchased 1 of Product 9
Customer 8 purchased 1 of Product 9
Customer 8 purchased 1 of Product 9

each thread was supposed to print out its respective arguments but instead, all three threads are printing the arguments of the last iteration.

For some reason the problem goes away if I add a sleep() call to the bottom of the for loop but I don't want it to sleep.

Would Appreciate any help.

Answer 1

myargs only exists until the end of the block in which it is created. When the loop pass is over, it ceases to exist, and it's undefined behaviour to access it.

Since the variable ceases to exist immediately after the thread is created, the code running in the thread attempts to access the variable after it has ceased to exist, and thus has undefined behaviour.

One solution is to extend the lifetime of the variable.

ThreadArgs myargs[ NO_OF_PRODUCTS ];

for ( int i = 0; i < NO_OF_THREADS; ++i ) {
   …
   myargs[i].… = …;
   …
   pthread_create( mythreads+i, NULL, thread_function, myargs+i )
   …
}

Another would be to use malloc to allocate the struct.

Another yet would be to ensure the thread has obtained and copied the data before moving on, which can be done through some form of synchronization.