CODEWITHSUNDEEP
cpthreadsposix
Štěpán Heller
Štěpán Heller

Reputation: 151

POSIX threads in C

I'm trying to understand how threads works. I have some examples from the school. In this one I have to figure out why this piece of code doesn't work properly. Its output is this:

Main: Creating thread 0
Main: Creating thread 1
Main: Creating thread 2
Main: Creating thread 3
Main: Creating thread 4
Main: Creating thread 5
Main: Creating thread 6
Main: Creating thread 7
Main: Creating thread 8
Thread 0: English: Hello World!
Thread 0: English: Hello World!
Thread 0: English: Hello World!
Thread 0: English: Hello World!
Thread 0: English: Hello World!
Thread 0: English: Hello World!
Thread 0: English: Hello World!
Thread 0: English: Hello World!
Thread 0: English: Hello World!

But every thread should say 'Hello World' in a different language. Here is my code. It works fine when the fourth parameter in function pthread_create is just (void *) t instead of the pointer. But I know that the right solution is with (void *) &t. Probably I'm dealing with some pointer problem but i just can't see the way...

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

#define NUM_THREADS 8

char *messages[NUM_THREADS + 1] = 
{
   "English: Hello World!",
   "French: Bonjour, le monde!",
   "Spanish: Hola al mundo",
   "Klingon: Nuq neH!",
   "German: Guten Tag, Welt!",
   "Russian: Zdravstvytye, mir!",
   "Japan: Sekai e konnichiwa!",
   "Latin: Orbis, te saluto!",
   "Cesky: Ahoj svete!"
};


void * helloThread ( void * threadid )
{
   int *id_ptr, taskid;

   sleep(1);
   id_ptr = (int *) threadid;
   taskid = *id_ptr;
   printf("Thread %d: %s\n", taskid, messages[taskid]);
   return(NULL);
}

int main(int argc, char *argv[])
{
   pthread_t threads[NUM_THREADS];
   int rc, t;

   for(t=0;t<=NUM_THREADS;t++) {
      printf("Main: Creating thread %d\n", t);
      rc = pthread_create(&threads[t], NULL, helloThread, (void *) &t );
      if (rc) {
         printf("ERROR; return code from pthread_create() is %d\n", rc);
         return (EXIT_FAILURE);
      }
  }

   pthread_exit(NULL);
   return ( 0 );
}

Upvotes: 2

Views: 625

Answers (3)

Kerrek SB
Kerrek SB

Reputation: 477040

There are several things wrong:

First off, you're overstepping bounds; the loop should say for(t = 0; t < NUM_THREADS; t++).

Second, you have to join or detach threads before ending the process, so say this at the end:

for(t = 0; t < NUM_THREADS; ++t) {
   pthread_join(threads[t], NULL);
}

Third, you are passing the same pointer (namely &t) to all threads. Not only is this giving you the wrong behaviour, but this is also exposing you to undefined behaviour on account of race conditions or dereferencing dangling pointers. Instead, give each thread its own memory:

int q[NUM_THREADS];  /* dedicated storage for each thread! */

for(t = 0; t < NUM_THREADS; ++t) {
   printf("Main: Creating thread %d\n", t);
   q[t] = t;
   rc = pthread_create(threads + t, NULL, helloThread, q + t);
   /* ... */
}

(Fourth, you shouldn't call pthread_exit in the way you do to terminate the main thread. It's unnecessary, and it precludes you from returning from main() in the usual way.)

Upvotes: 6

Freyja
Freyja

Reputation: 40804

&t passes a pointer to the variable t to the thread. Since every thread gets a pointer to the same variable (t), the value in every thread would be the same at any given time.

A simple solution:

pthread_t threads[NUM_THREADS];
int thread_ids[NUM_THREADS]; // array which will hold the ID of each thread
int rc, t;

for(t=0;t<NUM_THREADS;t++) { // fixed
  printf("Main: Creating thread %d\n", t);
  thread_ids[t] = t; // setting the thread id
  rc = pthread_create(&threads[t], NULL, helloThread, (void *) &thread_ids[t] );
  // notice that I use &threads_ids[t] instead of &t
  if (rc) {
    printf("ERROR; return code from pthread_create() is %d\n", rc);
    return (EXIT_FAILURE);
  }
}

Upvotes: 0

Brian Roach
Brian Roach

Reputation: 76898

You're passing a pointer to your loop iterator t in main().

When the loop increases the value of the iterator ... your running threads see whatever that is at the time. Since all your threads are going to wait for one second before doing anything and you're exiting from main() without waiting for the threads to complete, the thing you're pointing at is no longer valid and anything you see will be undefined behavior.

Upvotes: 1

Related Questions