Take the 2-minute tour ×

I'm trying to grasp the concepts of multithreading programming, and I think I'm doing well, but then I've found the following code for a simple echo server:


And I'm thinking that the code is wrong, because it uses the same main local variable to store the data socket for each incoming connection. In particular, Im concerned about this part of main():

while (1)
{   int client, addr_size = sizeof(addr);
    pthread_t child;

    client = accept(sd, (struct sockaddr*)&addr, &addr_size);
    printf("Connected: %s:%d\n", inet_ntoa(addr.sin_addr), ntohs(addr.sin_port));
    if ( pthread_create(&child, NULL, Child, &client) != 0 )
        perror("Thread creation");
        pthread_detach(child);  /* disassociate from parent */

As far as I understand, the variable client, local to the while loop, is allocated at exactly the same address in each iteration of the loop. So, when the first client is accepted, the thread receives &client, and when the second client is accepted, the value of client is overwritten with the new data socket, and this can have side-effects in the thread which is already running on the first client.

Observing the code of the function Child, which is the service thread, I can see that the argument is copied into a local variable:

void* Child(void* arg)
{   char line[100];
    int bytes_read;
    int client = *(int *)arg;

and probably the author thought that this copy allows him to later tamper with the main client variable, but IMHO this can cause a race condition. If a second client arrives while the first thread is copying this variable, the value copied can be corrupt.

Am I right?

share|improve this question

2 Answers 2

up vote 1 down vote accepted

Yes, you are correct. There are two obvious ways to fix this:

  1. Pass client to the thread instead of &client.

  2. Allocate a new integer on the heap and pass its address to the thread and let the thread free it when it's done with it.

share|improve this answer
in addition to this, accept() can fail. in that case client is not overwritten. –  qarma Nov 23 '13 at 15:38

Yes, you are right.

You can demonstrate that you're right by adding a long sleep before int client = *(int*)arg; and connecting to the server two times while the first client thread.

share|improve this answer

Your Answer


