2

I have a segmentation fault error in my program. I am practising with multithreading programs in POSIX C. I run these programs in FREEBSD system.

Here is my code:

#include <stdlib.h>
#include <stdio.h>
#include <pthread.h>
#include <unistd.h>
#include <sys/types.h>
#define NUM_THREADS 15

void *PrintHello(void *threadid)
{
        printf("Hello World from thread: %p \n", threadid);
        fflush(NULL);
        pthread_exit(NULL);
}

int main(int argc, char *argv[])
{
        int rtrn, *i;
        pthread_t threads[NUM_THREADS];

        for((*i)=0; (*i)<NUM_THREADS; (*i)++)
        {
                if( pthread_create(&threads[(*i)], NULL, PrintHello, (void *)i) != 0)
                {
                        printf("Error code %d detected while thread %d was being created.", rtrn, *i);
                }
        }
        return 0;
}

I had to turn i integer variable into pointer integer because this solved a error during compiling (line 22: warning: cast to pointer from integer of different size)

The compile error has been resolved but now when I run the program, it shows me this: Segmentation fault: 11 (core dumped)

Probably the previous warning has not been resolved but I don't know how to fix it...

chrisdal
  • 35
  • 5

3 Answers3

1

You need to allocate memory for the variable i. Right now it's a pointer pointing somewhere, but you definitely do not owe that piece of memory.

After a

i = calloc(1, sizeof(int));

You will have a valid piece of memory, initialized to 0 (thanks to calloc).

On the other end, if you will use i via a pointer you might have a funny behaviour depending how your OS is starting the threads. All threads will use the same address, so some of them might get the same value printed on the screen :) If you want to print out 10 different numbers via thread go with 0x90's answer :)

Ferenc Deak
  • 34,348
  • 17
  • 99
  • 167
1

I made some changes:

  1. Removed unnecessary header files.
  2. Declared a global array for the id. Better to share a global array or on heap than on stack.
  3. Used the array instead a pointer.

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

#define NUM_THREADS 15

void *PrintHello(void *threadid)
{
    printf("Hello World from thread: %d \n", *(int *)threadid);
    fflush(NULL);
    pthread_exit(NULL);
}

static int id[NUM_THREADS];

int main(int argc, char *argv[])
{
    int rtrn;
    pthread_t threads[NUM_THREADS];

    for(int i=0; i<NUM_THREADS; i++) {
         id[i] = i; 
         if (pthread_create(&threads[i], NULL, PrintHello, (void *)&id[i]))
             printf("Error code %d detected while thread %d was being created.", rtrn, i);

    }
    return 0;
}

Will output:

Hello World from thread: 0 
Hello World from thread: 1 
Hello World from thread: 2 
Hello World from thread: 3 
Hello World from thread: 4 
Hello World from thread: 5 
Hello World from thread: 6 
Hello World from thread: 7 
Hello World from thread: 8 
Hello World from thread: 9 
Hello World from thread: 10 
Hello World from thread: 11 
Hello World from thread: 12 
Hello World from thread: 13 
Hello World from thread: 14 
0x90
  • 39,472
  • 36
  • 165
  • 245
1

If all you want to do is pass an id to the calling thread you can do so by burying it in the void* parameter, and do so portably. Using the integer types like intptr_t, which is "the integer large enough to hold a data pointer" on your platform, I'm fairly sure this is closer to what you want.

Also, POSIX requires stdio functions on FILE* to be thread-safe, so you can lose the fflush() (which wasn't really helping anyway), though if you're going to search for why this is, it isn't obvious. Look in the documentation for flockfile(), and note the mention that all stdio functions that operate on FILE* (of which printf() does on stdout) should behave as if they invoke flockfileand funlockfile to acquire access to the stream.) There is no specification on the atomicity of the locking (it could be down to the character for all you know, in which case interleaving runs amok) but it should still be none-the-less safe.

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

#define NUM_THREADS 15

void *PrintHello(void *threadid)
{
    intptr_t id = (intptr_t)threadid;
    printf("Hello World from thread: %" PRIiPTR "\n", id);
    return NULL;
}

int main(int argc, char *argv[])
{
    pthread_t threads[NUM_THREADS];
    intptr_t i=0;
    int res;

    for(i=0; i<NUM_THREADS; ++i)
    {
        if( (res = pthread_create(threads+i, NULL, PrintHello, (void *)i)) != 0)
            printf("Error code %d detected while thread %" PRIiPTR " was being created.", res, i);
    }

    // wait for all to complete
    for (i=0;i<NUM_THREADS;++i)
        pthread_join(threads[i], NULL);

    return 0;
}

Output

Hello World from thread: 0
Hello World from thread: 1
Hello World from thread: 2
Hello World from thread: 3
Hello World from thread: 4
Hello World from thread: 5
Hello World from thread: 6
Hello World from thread: 7
Hello World from thread: 8
Hello World from thread: 9
Hello World from thread: 10
Hello World from thread: 11
Hello World from thread: 12
Hello World from thread: 14
Hello World from thread: 13

Finally, if you're doing a double-take on the odd looking PRIiPTR buried in the format strings it should be worth your time to read this question and answers. It is the portable mechanism for using the right "%" type to dump intptr_t as formatted data.

Community
  • 1
  • 1
WhozCraig
  • 65,258
  • 11
  • 75
  • 141