0

I'm doing a really easy program using pthreads but i keep getting a "Segmentation fault error" and I cannot understand why. The program is to be compiled in C language.

The program should create 3 threads, each of them should print its number and its calling thread number (in this case , i just want 3 threads to say "I'm thread (i) and the calling thread is 0 " since they are all created from the same thread)

The problem, I think, is with the malloc function used to allocate memory to be passed.

In case this is te problem, how can i solve it?

Should I create an array of 3 pointers before the "for" loop, and store in each pointer of this array the two variables i want to pass to the thread? How to actualy do it? I tried but it doesn't seem to work.

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

void *Son (void* arg)
{int *id=(int*)arg; //retrieve array
  sleep(id[1]+1);
  printf("I'm thread %d , created from %d! \n",id[1],id[0]);

free(arg);
 pthread_exit(0);
}




int main()
{pthread_t threads[3];
 int i;
 for (i=0;i<3;i++)
    {int *a= malloc(2*sizeof(int)); // dynamically allocate a variable which will be freed in the thread once executed
    a[0]=0;  //store i in the content of dynamically allocated a
    a[1]=i;
    pthread_create(&threads[i],NULL,Son ,(void*)a);
    }
int j;
for (j=0;j<3;i++)
    {pthread_join(threads[i],NULL);
    printf("Joined thread %d",j);
    }
printf("THREAD FINISHED\n");
return 0;
}
Spikatrix
  • 20,225
  • 7
  • 37
  • 83
Stefano Palmieri
  • 13
  • 3
  • Please don't cast to/from `void *` in C (like [with `malloc()`'s return value](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc), but also in general). Also, the indentation could be improved. – unwind May 26 '15 at 14:00
  • Sorrry for sonswithgrandsons, the actual program was just sons and still didn't work – Stefano Palmieri May 26 '15 at 14:05
  • @unwind Casting to/from `void*` could be helpful when trying to chain list elements of different types while trying emulate some kind of inheritance, couldn't it? – Eregrith May 26 '15 at 14:06
  • @Eregrith What? This isn't about "chaining" or "list elements", I don't understand your point. Sure there are places where casting is necessary, but they are quite rare. This code doesn't need casts, but still has them. – unwind May 26 '15 at 14:11
  • @unwind "*Please don't cast to/from void * in C (like with malloc()'s return value‌​, but also in general)*" this does not go well with "*Sure there are places where casting is necessary*". This is my point. – Eregrith May 26 '15 at 14:12
  • My professor told us it is necessary to cast to void when passing arguments to pthread create. Are u telling me it's not really necessary? – Stefano Palmieri May 26 '15 at 14:20
  • @StefanoPalmieri Yes, it's not necessary. In C you never have to cast to convert regular ("object") pointers to/from `void *`. Since pthreads is a C API, and you're writing C, you don't need to cast. – unwind May 26 '15 at 14:20
  • @Eregrith "In general" does not mean "always". For instance when dropping `const` you must cast. But, as I said, it's very rare (since dropping `const` is something you should basically never do). – unwind May 26 '15 at 14:22
  • Thanks, I will keep it in mind. Unfortunately my university professor thinks this is the good way, so in my exam I'll just cast even though it's a bad habit :-/ – Stefano Palmieri May 26 '15 at 14:57

1 Answers1

4

Here:

for (j=0;j<3;i++)
{
    pthread_join(threads[i],NULL);
    printf("Joined thread %d",j);
}

you probably wanted to use threads[j] instead of threads[i]. i, at this point is 4 and you are accessing an invalid memory address via the pthread_join. Also, you need to use j++ instead of i++.

Spikatrix
  • 20,225
  • 7
  • 37
  • 83
  • 1
    Shouldn't the lesson learned here be that you should write your loops like this: `for (int i=0; i!=limit; ++i)`? Creating the loop variable outside the loop was the fault that made this error possible in the first place. The general rule is to reduce the scope of variables as much as possible. – Ulrich Eckhardt May 26 '15 at 16:48
  • @UlrichEckhardt , Or simply stick to `i` instead of having multiple counter variables. – Spikatrix May 27 '15 at 02:40
  • I wouldn't advise that, @CoolGuy. It works just as well as applying the general rule of keeping variable scope small, but turning this general rule into a habit is what proactively prevents this kind of error (and other similar errors) in the future, too. – Ulrich Eckhardt May 27 '15 at 18:31