-1

I have written a program for creating number of threads. The number can be passed as a command line argument or default value is 5. The problem I am facing is printing the thread number passed as an argument to pthread writer function.

Below is my program: 

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

void *writerThread(void *args)
{
  int id = *((int *)args);
  printf("In thread %d\n", id);
}

int main(int argc, char *argv[])
{
  int numThreads;
  if(argc != 2)
    numThreads = 5;
  else
    numThreads = atoi(argv[1]);

  pthread_t *tId = (pthread_t*) malloc(sizeof(pthread_t) * numThreads);

  int i;
  for(i = 0; i < numThreads; i++)
  {
    pthread_create(&tId[i], NULL , writerThread, &i);
  }

  // wait for threads
  for(i = 0; i < numThreads; i++)
    pthread_join(tId[i], NULL);

  if(tId)
    free(tId);

  return 0;
}

Output:

In thread 2
In thread 0
In thread 3
In thread 2
In thread 0

Why 0 and 2 are coming two times ? Any help would be appreciated.

Bhawan
  • 2,441
  • 3
  • 22
  • 47
  • 1
    You must make sure each thread gets its own separate variable because there's no guarantee the that data pointed at will be the same as you intended by the time the thread gets to run. So, you need an array of thread numbers, and pass a different entry to each (`int thr_num[numThreads]; … { thr_num[i] = i; pthread_create(…, &thr_num[i]); }`. And, unless you think the number of threads will be very high, I'd use a VLA, or even a fixed size array, of the `pthread_t` structures — similar to the `thr_num` array suggested before. Or create an array of a structure with a `pthread_t` and an `int`. – Jonathan Leffler Dec 11 '17 at 05:32

2 Answers2

1

When you create the threads

pthread_create(&tId[i], NULL , writerThread, &i);

i continues its life... thus when you print *(int*)args in the thread. you get the current value ofi that keeps being used and changed in the main function.

You might want to allocate an array of integers that contain the thread id and store the id during the thread creation

int tids[numThreads];
for(i = 0; i < numThreads; i++) {
   tids[i] = i;
   pthread_create(&tId[i], NULL , writerThread, &tids[i]);
}

This way, each thread has an argument that points to its own personal data (in that case, an integer).

Déjà vu
  • 28,223
  • 6
  • 72
  • 100
1

To complement Ring Ø's answer, in your particular case of passing a small integral number to your thread routine, you could cast to an intptr_t type that argument, and code:

void *writerThread(void *args) {
  int id = (int)((intptr_t)arg);
  printf("In thread %d\n", id);
}

then 

for(i = 0; i < numThreads; i++) {
   pthread_create(&tId[i], NULL , writerThread, (void*)((intptr_t)i));
}

this is possible since pthread_create just passes an uninterpreted [pointer] word, which could actually be a cast from intptr_t, which is guaranteed to have the same size than pointers.

So you don't always need the last argument to pthread_create to be a valid (dereferencable) pointer.

BTW, on my Linux/Debian/x86-64 system, pointers (e.g. void*) and intptr_t have (like long) 8 bytes, but int is 4 bytes.

In the usual case, the last argument to pthread_create is often (as a rule of thumb) a unique pointer to some heap allocated malloc-ed zone (and you should later free it appropriately, usually after the corresponding pthread_join).

Notice that your code should test against failure of malloc and should have:

 pthread_t *tId = malloc(sizeof(pthread_t) * numThreads);
 if (tId == NULL) { perror("malloc tId"); exit(EXIT_FAILURE); };

since in the unlikely case that malloc failed your code exhibits undefined behavior because the first argument to pthread_create should be a valid address of some pthread_t.

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547