I'm struggling to solve this problem with my multi-threaded server. The server has to create a new thread for every connection and here's the code:
int main() {
int client, val = 1;
struct sockaddr_in saddr;
socklen_t slen = sizeof(struct sockaddr_in);
s_sock = socket(F_INET, SOCK_STREAM, 0);
memset( &saddr, 0, sizeof(struct sockaddr_in));
saddr.sin_family = AF_INET;
saddr.sin_port = htons(PORT);
inet_aton(HOST, &saddr.sin_addr);
setsockopt( s_sock, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(int) );
bind(s_sock, (struct sockaddr*) &saddr, sizeof(saddr));
listen(s_sock, MAX_CONN);
while(1) {
pthread_t thread;
client = accept( s_sock, (struct sockaddr*) &saddr, &slen);
printf("Accepted a client\n");
pthread_create(&thread, NULL, server, (void*) &client);
}
return 0;
}
The problem is if I start several clients simultaneously, first ones get lost and the last one finishes successfully. I guess it happens because of the variable thread
which gets rewritten when another connection is accepted.
Why does this happen and how to fix it?
It happens because client
gets rewritten, not thread
.
while(1) {
pthread_t thread;
client = accept( s_sock, (struct sockaddr*) &saddr, &slen);
printf("Accepted a client\n");
pthread_create(&thread, NULL, server, (void*) &client);
}
You have a race condition here. After the call to pthread_create
, this thread will loop back and change the value of client
with no synchronization. So what can the newly-created thread possibly do with &client
? It can't follow the pointer to get the descriptor because this thread is about to modify client
at some unpredictable time.
Imagine if this loop runs twice quickly with no opportunity for any other threads to run. You've now created two threads and passed them both &client
-- the very same value. You now have two threads working the same socket and one socket lost.
Do not pass a thread the address of some variable whose value you are about to change!