Search code examples
csocketsposix-select

Select blocks when trying to read activity from file descriptors


I am trying to create a server that accepts multiple clients. Here is a part of my code. My server needs to listen to two ports. The sockets that achieve this are fd_serv and fd_comm.

Using select and nonblock file descriptors, unfortunately does not give me the desired output. The problems are that select accepts the first client and then blocks, and doesn't read messages from other clients.

Also for some reason if I don't use the usleep(100000) command, my program succeeds accepting but doesn't go on.

Any help appreciated.

FD_ZERO(&set);
FD_SET(fd_serv, &set);
FD_SET(fd_comm, &set); 
while (1)
    {
    if (select (max_fd + 1, &set, NULL, NULL, NULL) < 0)
    {
        perror ("select");
        exit (EXIT_FAILURE);
    }

    for (i = 1; i < max_fd + 1; i++)
    {
        usleep(100000);
        if (FD_ISSET (i, &set))
        {
            if (i == fd_serv)       //Connection request on original socket.
            {
                new_serv = accept (fd_serv, (struct sockaddr *) &client_serv_addr, &sin_len_serv);
                if (new_serv < 0){ perror ("accept"); exit (EXIT_FAILURE); }
                fd_set_blocking(new_serv, 0);
                FD_SET(new_serv, &set);
                max_fd = max(max_fd, new_serv);
            }
            else if (i == fd_comm)  //Connection request on original socket.
            {
                new_comm = accept (fd_comm, (struct sockaddr *) &client_comm_addr, &sin_len_comm);
                if (new_comm < 0){ perror ("accept"); exit (EXIT_FAILURE); }
                fd_set_blocking(new_comm, 0);
                FD_SET (new_comm, &set);
                max_fd = max(max_fd, new_comm);
            }
            else if (i == new_comm) //Activity on already connected socket
            {
                if (read_command(fd_comm, i, start_time) == 1)
                {
                    close(i);
                    FD_CLR(i, &set);
                    return SHUTDOWN;
                }
            }
            else if (i == new_serv) //Activity on already connected socket
            {
                read_service(i);
                close(i);
                FD_CLR(i, &set);
            }
        }
    }
}

Solution

  • Your problem is here:

    FD_ZERO(&set);
    FD_SET(fd_serv, &set);
    FD_SET(fd_comm, &set); 
    while (1) 
    {
       [...]
    

    You are only setting up your fd_set once, but your fd_set will be modified by the select() call, so you need to set it up once per iteration of your loop. Try this instead:

    while (1) 
    {
       FD_ZERO(&set);
       FD_SET(fd_serv, &set);
       FD_SET(fd_comm, &set); 
       const int max_fd = (fd_serve > fd_comm) ? fd_serve : fd_comm;
       [...]
    

    ... after the select() call, the only select()-related thing you should be calling is FD_ISSET(). All the other stuff is pointless, since your additional FD_SET() and FD_CLR() calls are modifying an fd_set that you're just going to wipe clean with the FD_ZERO() call at the beginning of the next while-loop iteration.

    Also, as a side note: that usleep(10000) call should go away -- if you're calling select() correctly, sleeping for arbitrary time periods outside of select() is neither necessary nor desirable.