0

I'm having a segmentation fault problem with a networking program using threads to deal with each new connection.

MAX_PEERS is defined above as 10.

...
int iret[MAX_PEERS];
pthread_t thread[MAX_PEERS];


(void) signal(SIGCHLD, reaper);
printf("before while\n");
int i = 0;
while(1) {
    if(i>MAX_PEERS-1){break;}
    client_len = sizeof(client);
    new_sd = accept(sd, (struct sockaddr *)&client, &client_len);
    if(new_sd < 0){
    fprintf(stderr, "Can't accept client \n");
    exit(1);
  }
    printf("before thread\n");
    iret[i] = pthread_create(&thread[i], NULL, connection, (void*) new_sd);
    if(iret != 0){
        printf("thread[%d] not generated!\n", i);
    }
    i++;
    printf("end of while\n");
}
....

and the function "connection" beggins as follows

void *connection(void *sdd)
{
    int sd =* (int *) sdd; 
...

When the client tries to connect to the server I get a segmentation fault.

> ./server 20011                  
before while
before accept
after accept
before thread
./server: zsh: segmentation fault  ./server 20011
>

It prints "before accept" before the client connects, and after the client connects it prints the rest.

Am I creating the threads correctly? Any ideas?

Thanks,

2
  • You should pass &new_sd as arg to pthread_create
    – rednaks
    Nov 11, 2014 at 23:59
  • this line: if(i>MAX_PEERS-1){break;} moves execution to the line: printf("before thread\n"); and the very next line is using that 'i' value as an index into the array of '&thread[i],' which results in undefined behaviour, in this case, with a seg fault event Nov 12, 2014 at 2:54

1 Answer 1

0

accept() returns an integer, so new_sd must be an integer. When you create a new thread you cast new_sd to a void pointer. But when you get that pointer in the function, instead of casting back to int you cast it to a pointer to int and dereference it, which causes the seg fault.

Instead of using possibly incorrect casting, rather pass an integer. Assuming that new_sd will go out of scope, allocate space for an integer and pass it to your thread.

int* new_sdp = malloc( sizeof( *new_sdp )) ;
*new_sdp = new_sd ;

iret[i] = pthread_create(&thread[i], NULL, connection, new_sdp);

and in the thread:

void *connection(void *sdd)
{
    int sd = *(int *)sdd; 
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.