HPC-Tutorials icon indicating copy to clipboard operation
HPC-Tutorials copied to clipboard

Suggestion on PThreads (passing argument, example3)

Open j7168908jx opened this issue 3 years ago • 3 comments

As from the title, in PThreads tutorial, Passing Arguments to Threads, Example 3

Example 3 - Thread Argument Passing (Incorrect)
This example performs argument passing incorrectly. It passes the address of variable t, which is shared memory space and visible to all threads. As the loop iterates, the value of this memory location changes, possibly before the created threads can access it.
int rc;
long t;

for(t=0; t<NUM_THREADS; t++) 
{
   printf("Creating thread %ld\n", t);
   rc = pthread_create(&threads[t], NULL, PrintHello, (void *) &t);
   ...
}

with output

Creating thread 0
Creating thread 1
...
Creating thread 7
Hello from thread 140737488348392
Hello from thread 140737488348392
...
Hello from thread 140737488348392
Hello from thread 140737488348392

It looks like the program prints the address of the variable t, which might be inconsistent with what we want to illustrate --- "the value of this memory location changes". Can we adjust this to something like the following?

void *PrintHello(void *threadid) {
    long * tid;
    tid = (long *)threadid;
    printf("Hello World! It's me, thread #%ld!\n", *tid);
    pthread_exit(NULL);
}

int main() {
    pthread_t threads[NUM_THREADS];
    int rc;
    long t;
    for (t = 0; t < NUM_THREADS; t++)
    {
        printf("In main: creating thread %ld\n", t);
        /* Don't try to access memory location of t! 
         * It constantly changes as the loop goes! 
         * Possibly before the created threads can access it
         */
        rc = pthread_create(&threads[t], NULL, PrintHello, (void *) &t);
        if (rc)
        {
            printf("ERROR; return code from pthread_create() is %d\n", rc);
            exit(-1);
        }
    }

    /* Last thing that main() should do */
    pthread_exit(NULL);
    return 0;
}

So the output goes like

In main: creating thread 0
In main: creating thread 1
Hello World! It's me, thread #1!
In main: creating thread 2
Hello World! It's me, thread #2!
In main: creating thread 3
Hello World! It's me, thread #3!
In main: creating thread 4
Hello World! It's me, thread #4!
Hello World! It's me, thread #5!

which illustrates that the value t had been t++ed before the created thread read the location.

Thanks!

j7168908jx avatar Jul 14 '22 08:07 j7168908jx

I can create a PR if this suggestion is accepted. Thanks!

j7168908jx avatar Jul 14 '22 08:07 j7168908jx

Hi, the author wrote this example (marked as Incorrect) meant to show us that passing an local variable as thread's argument is Incorrect because when the for loop end, the local variable t's memory is decallocated and maybe allocated for other usage, accessing this address is not safe when for loop end, so the author print the t's address in every thread that created explicitly to show that all thread's arugment *arg is acutally the address of t in the for loop.

Zard-C avatar Aug 10 '23 07:08 Zard-C

Also, your modification seems not right, because you pass an local variable t to every thread while adding t by 1, according to the context, we should passing every thread a t, not every thread shares one value.

If you want to write the correct version, here's a modification


int main() {
    pthread_t threads[NUM_THREADS];
    int rc;
    long t[NUM_THREADS];
    for (t = 0; t < NUM_THREADS; t++)
    {
        printf("In main: creating thread %ld\n", t);
        t[i] = i;
        rc = pthread_create(&threads[t], NULL, PrintHello, &t[i]); /* you don't need to add (void*) */
        if (rc)
        {
            printf("ERROR; return code from pthread_create() is %d\n", rc);
            exit(-1);
        }
    }

    /* Last thing that main() should do */
    pthread_exit(NULL);
    return 0;
}

Zard-C avatar Aug 10 '23 07:08 Zard-C