pthread_join() causing segmentation fault

Question

I'm working on understanding threads and have run up against this segmentation fault that I can't seem to correct. I have narrowed down the error to the pthread_join() function, but can't seem to go any further. My understanding is that the for loop needs to be the same as the one for the pthread_create() and that the variable that is passed in is the dereferenced pointer being passed into the pthread_exit() function. Any direction would be greatly appreciated.

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

//void *runner(void *param);

void *pFactor(void *param)
{
        int *pFArray = malloc(sizeof(int) *32);
        int myNum = atoi(param);
        int count = 0;

        printf("I'm getting here");

        while (myNum % 2 == 0)
        {
                pFArray[count] = 2;
                myNum = myNum/2;
                count++;
        }

        for (int i = 3; i < sqrt(myNum); i += 2)
        {
                while (myNum % i == 0)
                {
                        pFArray[count] = i;
                        myNum = myNum/i;
                        count++;
                }
        }

        if (myNum > 2)
        {
                pFArray[count] = myNum;
        }

        //int *arrPtr = (int*) malloc(10);
        //arrPtr = pFArray;
        //return (void *) arrPtr;
        //return fprint("I made it to the end of the pFactor function")
        pthread_exit(pFArray);

}

int main(int argc, char *argv[])
{
        pthread_t tid[argc];
        pthread_attr_t attr;

        if (argc == 0)
        {
                fprintf(stderr, "usage: a.out <integer value>\n");
                return -1;
        }
        if (argc < 0)
        {
                fprintf(stderr, "%d must be >= 0\n", atoi(argv[1]));
                return -1;
        }

        /* Get the default attributes */
        pthread_attr_init(&attr);
        /* Create the thread */
        for (int i = 1; i < argc; i++)
        {
                tid[i] = i;
                pthread_create(&tid[i], &attr, pFactor, &argv[i]);
                printf("still working\n");
        }
        printf("still working\n");
        /* Wait for the thread to exit */
        for (int i = 1; i < argc; i++)
        {
                tid[i] = i;
                void *my_Ptr;
                printf("still working 1\n");
                pthread_join(tid[i], &my_Ptr);
                printf("still working 2\n");
                for (;;)
                {
                        printf("%d", atoi(argv[i]));
                        printf(": %d",((int*) my_Ptr)[i-1]);
                }
                //printf("%d", atoi(argv[i]));
                //printf(": %d",((int*) my_Ptr)[i-1]);
        } /*This is where you want to pass in the sum variable*/
}

Why are you assigning to tid[i]? In either loop? pthread_create() fills in the appropriate value, and those are the values you must later pass to pthread_join(). — John Bollinger

John Bollinger John Bollinger · Accepted Answer · 2019-02-27T23:11:26

Running your program under valgrind confirms your diagnosis that the issue occurs when you call pthread_join. That could also be determined by running it in a debugger, but Valgrind is often especially helpful with segfaults, because its memory-use analysis often helps identify the cause of the problem, too.

In this case, however, the primary issue is clear: you call pthread_join() with an invalid first argument. You seem to have a bit of a misunderstanding:

My understanding is that the for loop needs to be the same as the one for the pthread_create()

No, not necessarily. The only significant association with pthread_create is that the first argument, a thread ID, needs to be a value that was stored by pthread_create(), identifying a thread that has neither been detached nor already joined. It may be appropriate to perform your joins in a loop similar to the one in which you launch the threads, but that's driven by your application's particular requirements. Typically, though, those requirements include that all the worker threads complete before the main thread proceeds past some point, and to achieve that, each one must be joined.

and that the variable that is passed in is the dereferenced pointer being passed into the pthread_exit() function.

Knowing as I do what the behavior of pthread_join() is, I can just about squeeze those words into a shape that fits. Myself, I would say something more like, "The second argument, if non-null, is a pointer to a location in which to write the thread function's return value, or the value it passed to pthread_exit()."

Anyway, the main problem is what I already expressed in comments: in your join loop, you are overwriting the thread IDs provided by pthread_create, and passing your arbitrary values to pthread_join. These do not identify your threads, and in the event that your pthreads implementation is like mine, the bad thread IDs end up in fact being invalid pointers. What you seem not to appreciate is that it is pthread_create that generates and assigns thread IDs. That's why you have to pass it an address in which to store the ID. As a corollary, it is not useful to initialize the thread ID variable before passing its address to pthread_create.

Cleaning up that issue and running your program under valgrind again reveals that you have an additional issue in your thread function, as also referenced in your other answer. This is happening in part because your fourth argument to pthread_create is mismatched with your thread function's usage of its argument.

The fourth argument to pthread_create is passed straight through to the thread function. You are passing &argv[i], which is the address of a char *, a.k.a. a char **, automatically converted to a void *. Your thread function passes its argument directly to atoi(), with an automatic conversion to type char *. Note well that char * != char **. I think you want the fourth argument to pthread_create to be simply argv[i]. At least that has a type matching your use.

Having made that correction, too, I find that your program ends up in the infinite loop inside your pthread_join loop. I'm not sure why you're looping there -- it appears wholly gratuitous.

pthread_join() causing segmentation fault

2 Answers