my code should be printing 0 1 2 3 4 but why output is from 1 to 5. every time is re-run code it change output but range remains same 1-5 output i am getting: 1 2 3 5 5, 1 2 2 5 5, 5 5 5 5 5 expected output is : 0 1 2 3 4 can anyone tell why i am not getting expected output

#include <stdio.h>
#include <pthread.h>
void* printInt(void*ptr)
{
    int*iptr=(int*)ptr;
    printf("%d\n",*iptr);
}
int main()
{
    pthread_t tid[5];
    pthread_attr_t attr;
    pthread_attr_init(&attr);
    for(int i=0;i<5;i  )
    {
        pthread_create(&tid[i],&attr,printInt,(void*)&i);
    }
    for(int i=0;i<5;i  )
    {
        pthread_join(tid[i],NULL);
    }

    return 0;
}

CodePudding user response：

In this call

pthread_create(&tid[i],&attr,printInt,(void*)&i);

you are passing a pointer to the same local variable i that can be changed in the loop before a thread starts and moreover the program can have undefined behavior because the pointed variable can be unalive when a thread will try to access it.

CodePudding user response：

Unfortunately, your code has two race conditions, only one of which can be fixed [easily].

1. In main, passing i by pointer allows each thread function to receive [seemingly] random values. That's because the pointer given to each thread points to the same variable (i.e. i). For example, the value passed to thread 0 could be 1 because main executes its loop faster than thread 0 can be started.

2. Even if we fix the above [see below for a way], just because the threads are created [via pthread_create] in the order: 0,1,2,3,4 there is no guaranteed that they will execute/print in that same order. For example, they could execute in the order: 3,0,2,1,4

To fix the first issue, in main, we could pass i by value:

pthread_create(&tid[i],&attr,printInt,(void *) (uintptr_t) i);

And, a corresponding change in printInt:

int i = (uintptr_t) ptr;
printf("%d\n",i);

However, the second issue can't be fixed without some sort of synchronization between the threads, which defeats the purpose of having separate threads.

There are different [equally messy] ways to do this.

For example, main and the threads could synchronize via one or two global variables that are stored/fetched via (e.g.) stdatomic.h primitives to have main "grant" access to each thread in order. And, the thread would have to acknowledge completion via the other global.

Or, mutexes and semaphores could be used.

Here is a [crude] method of sync, using atomics. We create a per-task struct to hold the data for a given thread:

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

struct tsk {
    pthread_t tsk_tid;
    int tsk_i;
    int tsk_sync;
};

void *
printInt(void *ptr)
{
    struct tsk *tsk = ptr;

    // wait for main to give us the "go"
    while (1) {
        if (atomic_load(&tsk->tsk_sync) == 1)
            break;
        usleep(1);
    }

    printf("%d\n", tsk->tsk_i);
    fflush(stdout);

    // tell main we're done
    atomic_store(&tsk->tsk_sync,2);

    return (void *) 0;
}

int
main(void)
{
    struct tsk tsklist[5] = { 0 };
    struct tsk *tsk;
    pthread_attr_t attr;

    pthread_attr_init(&attr);

    for (int i = 0; i < 5; i  ) {
        tsk = &tsklist[i];
        tsk->tsk_i = i;
        atomic_store(&tsk->tsk_sync,0);
        pthread_create(&tsk->tsk_tid, &attr, printInt, tsk);
    }

    // grant each thread its turn, and wait for it to acknowledge
    for (int i = 0; i < 5; i  ) {
        tsk = &tsklist[i];
        atomic_store(&tsk->tsk_sync,1);
        while (atomic_load(&tsk->tsk_sync) == 1)
            usleep(1);
    }

    for (int i = 0; i < 5; i  ) {
        tsk = &tsklist[i];
        pthread_join(tsk->tsk_tid, NULL);
    }

    return 0;
}

There are [probably] better ways to sync the threads.

In this method, the main thread grants "access" and waits for the thread to "complete".

An alternate would be that once the main thread grants access to the first thread (e.g. thread 0), then the thread could run and it could grant access to the next thread in the list (e.g. thread 0 grants access to thread 1, etc.):

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

struct tsk {
    pthread_t tsk_tid;
    int tsk_i;
    int tsk_sync;
};

int owner = -1;

void *
printInt(void *ptr)
{
    struct tsk *tsk = ptr;

    // wait for somebody to give us the "go"
    while (1) {
        if (atomic_load(&owner) == tsk->tsk_i)
            break;
        usleep(1);
    }

    printf("%d\n", tsk->tsk_i);
    fflush(stdout);

    // say we're done and start the next thread
    atomic_store(&owner,tsk->tsk_i   1);

    return (void *) 0;
}

int
main(void)
{
    struct tsk tsklist[5] = { 0 };
    struct tsk *tsk;
    pthread_attr_t attr;

    pthread_attr_init(&attr);

    for (int i = 0; i < 5; i  ) {
        tsk = &tsklist[i];
        tsk->tsk_i = i;
        pthread_create(&tsk->tsk_tid, &attr, printInt, tsk);
    }

    // start first thread
    atomic_store(&owner,0);

    for (int i = 0; i < 5; i  ) {
        tsk = &tsklist[i];
        pthread_join(tsk->tsk_tid, NULL);
    }

    return 0;
}

CodePudding user response：

@Vlad has already given you the reason for your problem. One solution is to pass keep the values to be passed in in an array.

int param[5];
for (int i = 0; i < 5;   i)
{
    param[i] = i;
    pthread_create(&tid[i], &attr, printInt, (void*)&param[i]);
}

That way the value doesn't change before the thread starts running.