Search code examples
chttpnetwork-programmingpthreadswebserver

Calling `accept` syscall in loop creates corrupted sockets


I'm trying to build myself a web app in C. So far, I've got the HTTP server working, but for some reason, when I have things like HTML files that require other CSS, and JavaScript files, the browser (Firefox) will do several requests very fast, causing the socket connection between the server and the client to become corrupted. I've tried many different things to get the HTTP server to work without generating corrupted TCP connections.

Currently, I have a loop that calls accept, and creates a new thread for each new connection:

for(;;)
{
    int newsockfd = accept(sockfd, (struct sockaddr *)&host_addr, (socklen_t *)&host_addrlen);
    if(newsockfd < 0)
    {
        sendf(stderr, LOG_ERROR, "Failed to accept connection...\n");
        continue;
    }
    pthread_t thread;
    if(pthread_create(&thread, NULL, handler, (void *)&newsockfd) != 0)
    {
        sendf(stderr, LOG_ERROR, "Failed to create a new thread...\n");
        return -4;
    }
}

I create the thread, while sending the socket file descriptor (that represents the new connection between the client and server) to the new thread.

The function that handles the newly created thread looks like this:

void * handler(void * argument)
{
    int sockfd = *((int *)argument);

    struct sockaddr_in client_addr;
    int client_addrlen = sizeof(client_addr);

    int sockn = getpeername(sockfd, (struct sockaddr *)&client_addr, (socklen_t *)&client_addrlen);
    if(sockn < 0)
    {
        sendf(stderr, LOG_ERROR, "Failed to get client's address, got \x1b[35msockn\x1b[0m \x1b[33;3m%d\x1b[0m...\n", sockn);
        pthread_exit(NULL);
    }

    char * input = malloc(sizeof(char) * BUFFER_SIZE);

    int valread = read(sockfd, input, BUFFER_SIZE);
    if(valread < 0)
    {
        sendf(stderr, LOG_ERROR, "Failed to read from socket, got \x1b[35mvalread\x1b[0m \x1b[33;3m%d\x1b[0m...\n", valread);
        free(input);
        pthread_exit(NULL);
    }

    char * method = malloc(sizeof(char) * BUFFER_SIZE);
    char * url = malloc(sizeof(char) * BUFFER_SIZE);
    char * version = malloc(sizeof(char) * BUFFER_SIZE);
    sscanf(input, "%s %s %s", method, url, version);
    sendf(stdout, LOG_DEBUG, "Client \x1b[33;3m%s:%u\x1b[0m has sent a \x1b[33;3m%s\x1b[0m request to \x1b[33;3m%s\x1b[0m!\n", inet_ntoa(client_addr.sin_addr), ntohs(client_addr.sin_port), method, url);

    ...

When the connection is corrupted, I get:

[ERROR]: Failed to get client's address, got sockn -1...

With some debugging tools, I then check and see that the first bytes in method, url, and verison are set to NULL. The function getpeername also returns -1. The function sendf is just basic printf-like function, but doesn't seem to be the cause of the issue.

Anyone spot any issue here?


Solution

  • You are passing your thread handler a pointer to a local variable newsockfd which is no longer in scope by the time the thread begins running.

    You need to either:

    • allocate a new int dynamically which the handler will free, eg:
    int *newsockfd = malloc(sizeof(int));
    if (!newsockfd) { ... }
    *newsockfd = ...;
    ...
    pthread_t thread;
    if (pthread_create(..., newsockfd) != 0) {
        free(newsockfd);
        ...
    }
    
    void * handler(void * argument)
    {
        int *sockfd = (int*)argument;
        ...
        free(sockfd);
    }
    
    • simply pass the value of newsockfd instead of a pointer to it, eg:
    int newsockfd = ...;
    ...
    pthread_t thread;
    pthread_create(..., (void *)(intptr_t)newsockfd)
    
    void * handler(void * argument)
    {
        int sockfd = (int)(intptr_t)argument;
        ...
    }