Identifying threads on C

Question

So I'm learning how to use threads on C, and I wrote down this code.

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

#define number 3

void *funct(void *tid){
    printf("Thread %d\n", *((int*)tid));
    sleep(10);
    printf("Thread %d finishes\n", *((int*)tid));
    pthread_exit(NULL);
}

int main() {
    pthread_t threads[number];
    int i, status;
    printf("Pid: %d\n", getpid());
    for (i = 0; i < number; i++) {
        printf("Thread number %d\n", i);
        status = pthread_create(&threads[i], NULL, funct, (void*)&i);

        if(status != 0){
            printf("Error en la creación del hilo. La función devuelve %d\n", status);
            exit(-1);
        }
    }

    for (i = 0; i < number; i++) {
        pthread_join(threads[i], NULL);
    }
    return 0;
}

My problem is that I want to identify them on funct with their number, but sometimes the output is wrong.

Pid: 10142
Thread number 0
Thread number 1
Thread number 2
Thread 2
Thread 1
Thread 0
Thread 0 finishes
Thread 0 finishes
Thread 0 finishes

It shouldn't be printing Thread 0 finishes 3 times. I suppose the content of tid changes between the calls to the threads and that's why it print another values. How can I fix this?

You are passing the address of the same local variable to thread....so why do you expect a different output?... — LPs, Nov 03 '16 at 09:49
Instead of `(void*)&i`, you probably want to pass `&threads[i]`, and treat `tid` as `pthread_t *` in the callback. As long as the main thread outlives the created threads (which is ensured with `pthread_join` in your code), this is safe despite `threads` being stack-allocated. — user4815162342, Nov 03 '16 at 09:50

score 3 · Answer 1 · answered Nov 03 '16 at 09:46

You have a data race because you are passing the address of same variable i to all the thread functions. You can instead use a temporary array to pass the thread "numbers".

   int thr_id[number];
   for (i = 0; i < number; i++) {
        thr_id[i] = i;
        printf("Thread number %d\n", i);
        status = pthread_create(&threads[i], NULL, funct, &thr_id[i]);
        ...

Also, note the cast to the void pointer isn't necessary in C.

score 1 · Answer 2 · edited May 23 '17 at 12:00

1

You have a data race since you pass the variable of a single variable to different threads, so the value of that variable will change as more threads are created.

On most/typical implementations, you could squeeze the integer number into the pointer value itself, and do away with having an integer array just for this:

status = pthread_create(&threads[i], NULL, funct, (void *) (intptr_t) i);

And then inside the function just cast the received pointer back to an integer:

printf("Thread %d\n", (int) (intptr_t) tid);

The casts to/from intptr_t should help avoid warnings when the size of void * is larger than the size of int. See this question for instance.

edited May 23 '17 at 12:00

Community

1
1

answered Nov 03 '16 at 09:55

unwind

391,730
64
469
606

1

_warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]_.... ;) – LPs Nov 03 '16 at 09:57
1

There's no guarantee that integers and pointer have the same size. There's no way to directly pass like this. It could be cast to `uintptr_t` but that's also not perfect as it's *implementation-defined* at best. – P.P Nov 03 '16 at 10:00
@usr Right, considering the use case I think it's fine if `int` is (much) smaller than `void *`, that'd make sure all of the number gets across. Since the numbers are small, there's no real risk of running out of precision. – unwind Nov 03 '16 at 10:06
@usr I agree with you. Ironically, you solution is also implementation-defined. – 2501 Nov 03 '16 at 10:11
@2501. Indeed. I could change it to use `malloc` if you think it's dangerous ;-) – P.P Nov 03 '16 at 10:14

Identifying threads on C

2 Answers2