Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion on PThreads (passing argument, example3) #13

Open
j7168908jx opened this issue Jul 14, 2022 · 3 comments
Open

Suggestion on PThreads (passing argument, example3) #13

j7168908jx opened this issue Jul 14, 2022 · 3 comments

Comments

@j7168908jx
Copy link

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
Copy link
Author

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

@Zard-C
Copy link

Zard-C commented Aug 10, 2023

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
Copy link

Zard-C commented Aug 10, 2023

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;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants