I’m trying to learn socket programming and multithreading in C. Currently I’m working on a server that manages a chat room that up to 16 clients can connect to.

I have this code snippet:

    struct serverinfo server;
    struct receiveinfo ri[MAXCLIENTS];

    // Populate the serverinfo structure with zeros (0 = no client connected on that socket)
    server.si_serverfd = serverfd;
    for (i = 0; i < MAXCLIENTS; i++) {
        server.si_clientfds[i] = 0;
    }
    server.si_size = 0;
    server.si_terminate = 0;
    server.si_new = -1; // -1 = no new client connected

Here I’m creating and initializing the serverinfo structure (structure that holds server information) and receiveinfo structure, here’s the structure definitions:

struct serverinfo {
    int si_serverfd;
    int si_clientfds[MAXCLIENTS];
    int si_size;
    int si_terminate; // 0 or 1 (0 = don't terminate, 1 = Terminate)
    int si_new; // -1 or file descriptor value of a newly connected client
};

struct receiveinfo {
    int ri_senderfd;
    int ri_clientfds[MAXCLIENTS];
    int ri_size;
    int *ri_terminate;
};

Here’s the important part

I’m creating a thread that handle’s new connections like this pthread_create(&accept_t, NULL, handle_new, (void *)&server);

The handle_new() function looks like this:

void *handle_new(void *arg) {
    struct serverinfo *si = (struct serverinfo *) arg;
    int tempfd;
    char addrstr[INET6_ADDRSTRLEN];

    struct sockaddr_storage clientaddr;
    socklen_t addrlen = sizeof(clientaddr);

    struct sockaddr_in *ipv4;
    struct sockaddr_in6 *ipv6;

    int i;

    while (si->si_terminate != 1) {
        tempfd = accept(si->si_serverfd, (struct sockaddr *)&clientaddr, &addrlen);
        if (tempfd == -1) {
            printf("handle_new(): accept(): errorn");
            si->si_terminate = 1;
            break;
        }

        if (clientaddr.ss_family == AF_INET) {
            ipv4 = (struct sockaddr_in *)&clientaddr;
            inet_ntop(AF_INET, &ipv4->sin_addr, addrstr, sizeof(addrstr));
        } else if (clientaddr.ss_family == AF_INET6) {
            ipv6 = (struct sockaddr_in6 *)&clientaddr;
            inet_ntop(AF_INET6, &ipv6->sin6_addr, addrstr, sizeof(addrstr));
        } else {
            printf("Invalid address familyn");
            si->si_terminate = 1;
            break;
        }

        printf("+ %s connectedn", addrstr);
        si->si_size++;
        for (i = 0; i < MAXCLIENTS; i++) {
            if (si->si_clientfds[i] == 0)
                si->si_clientfds[i] = tempfd;
        }
        printf("tempfd: %dn", tempfd);
        si->si_new = tempfd;
        printf("si_new: %dn", si->si_new);
    }
    printf("Terminating...n");
    return NULL;
}

I’m accept()-ing a new connection and then setting the si->si_new value like this: si->si_new = tempfd. I previously set it to -1, it should be different in the main thread as well since I passed a pointer to the server structure to the handle_new() function, or so I thought.

Back in the main thread, when I check whether server.si_new is NOT -1:

while (server.si_terminate != 1) { // if server is not terminating
        if (server.si_new != -1) { // if new client connected
            printf("Not -1n");
            // ...

This if statement doesn’t work and I have no idea why.

I’m still quite new to multithreading and memory management among multiple threads, so obviously I’m doing something wrong.

The reason I have no protection against race conditions is because I thought it was unnecessary since I’m never modifying the same variable in multiple threads, only the accept_t (handle_new()) thread modifies the serverinfo stucture, other threads only read from it.

Sorry if this is a stupid question.

Any responses, suggestions, opinions and recommendations will be much appreciated.

Thank you in advance for all your answers.