Search code examples
ctimerposix-select

C: Part of the code doesn't execute under select()


I have something like this:

#define QUIT_TIME 5
int main(int argc, char **argv) {
        //... SOCKETS STUFF ....
    fdmax = parentfd;


    while (notdone) {

        //Set the timers
        waitd.tv_sec = 1;
        waitd.tv_usec = 0;

        FD_ZERO(&tempreadfds);
        FD_ZERO(&tempwritefds);

        FD_ZERO(&readfds);          /* initialize the read fd set */
        FD_ZERO(&writefds);         /* initialize the write fd set */

        FD_SET(parentfd, &readfds); /* add listener socket fd */
        FD_SET(0, &readfds);        /* add stdin fd (0) */

        tempreadfds = readfds; //make a copy
        tempwritefds = writefds; //make a copy

        if (select(fdmax+1, &tempreadfds, &tempwritefds, (fd_set*) 0, &waitd) < 0) {
            error("ERROR in select");
        }

        for(i = 1; i <= fdmax; i++) {

            if(FD_ISSET(i, &readfds)) {
                if(i == parentfd) {
                //This is a new connection
                childfd = accept(parentfd, (struct sockaddr *) &clientaddr, &clientlen);
                if (childfd < 0)
                    error("ERROR on accept");

                InitializeDataStructures(childfd);

                FD_SET(childfd, &readfds); //add to the master set
                if(childfd > fdmax)
                    fdmax = childfd;
            } else {
                //Existing connection
                if((nBytes = read(i, connections[i].buffer, MAXBUFFER)) <= 0) {
                    if(nBytes == 0) {
                        //Connection closed
                        printf("Socket %d hung up\n", read_write_loop);
                    } else {
                        error("\nReceive error\n");
                    }

                    FD_CLR(i, &readfds);
                } else {
                    //We have some data from the connection
                                            //... Manipulate the buffer
                    //Handle the message
                }
            }
            }

            if(FD_ISSET(i, &writefds)) {
                                .....
                FD_CLR(i, &writefds);
            }

            //Timer checking
            if(connections[i].active) {
                gettimeofday(&TimeNow, NULL);
                timeval_diff(&Interval, &TimeNow, &connections[i].TimeConnected);
                printf("*_*_*__*_*_*__*_*_*_*_* difference is %ld seconds, %ld microseconds\n",
                         Interval.tv_sec,
                         Interval.tv_usec
                        );
                if(Interval.tv_sec >= QUIT_TIME) {
                    printf("Timer elapsed!!\n");
                }
            }

        }
    }

    /* clean up */
    printf("Terminating server.\n");
    close(parentfd);
    return 0;
}

void InitializeDataStructures(int i) {

    clients[i].active = YES;
    clients[i].fd = i;
    //Initialize other members of the structure
}

long long timeval_diff(struct timeval *difference, timeval *end_time, struct timeval *start_time) {
      struct timeval temp_diff;

      if(difference==NULL)
        difference=&temp_diff;

      difference->tv_sec =end_time->tv_sec -start_time->tv_sec ;
      difference->tv_usec=end_time->tv_usec-start_time->tv_usec;

      while(difference->tv_usec<0)
      {
        difference->tv_usec+=1000000;
        difference->tv_sec -=1;
      }

      return 1000000LL*difference->tv_sec + difference->tv_usec;

    }

I was expecting that the "Timer elapsed" line will be printed every at least once (The TimeConnected was initialized into one of the if conditions) during execution, but for some reason, it never prints out. I thought my while loop should keep printing it... Anyone know if I'm messing up somewhere?

EDIT: Actually, I'm using the timer to disconnect the time after a timeout. I just observed that it prints "Timer elapsed" if another client connects to the server. I did pass the final parameter to select but am not sure why it is not having any effect.

Thanks to bdk!! If you're interested in knowing the "silly" bug I had in this code, read the discussion below in detail... It was an obvious mistake that I overlooked... all because of one sentence in the tutorials: "select modifies your original descriptors".

List of changes:

  • Notice that a set of FD_ZERO statements were wrongly placed inside the while loop
  • FD_ISSET was being passed readfds and writefds instead of tempreadfds and tempwritefds...

WORKING CODE:

#define QUIT_TIME 5
int main(int argc, char **argv) {
        //... SOCKETS STUFF ....
    fdmax = parentfd;


    FD_ZERO(&readfds);          /* initialize the read fd set */
    FD_ZERO(&writefds);         /* initialize the write fd set */

    while (notdone) {

        //Set the timers
        waitd.tv_sec = 1;
        waitd.tv_usec = 0;

        FD_ZERO(&tempreadfds);
        FD_ZERO(&tempwritefds);


        FD_SET(parentfd, &readfds); /* add listener socket fd */
        FD_SET(0, &readfds);        /* add stdin fd (0) */

        tempreadfds = readfds; //make a copy
        tempwritefds = writefds; //make a copy

        if (select(fdmax+1, &tempreadfds, &tempwritefds, (fd_set*) 0, &waitd) < 0) {
            error("ERROR in select");
        }

        for(i = 1; i <= fdmax; i++) {

            if(FD_ISSET(i, &tempreadfds)) {
                if(i == parentfd) {
                //This is a new connection
                childfd = accept(parentfd, (struct sockaddr *) &clientaddr, &clientlen);
                if (childfd < 0)
                    error("ERROR on accept");

                InitializeDataStructures(childfd);

                FD_SET(childfd, &readfds); //add to the master set
                if(childfd > fdmax)
                    fdmax = childfd;
            } else {
                //Existing connection
                if((nBytes = read(i, connections[i].buffer, MAXBUFFER)) <= 0) {
                    if(nBytes == 0) {
                        //Connection closed
                        printf("Socket %d hung up\n", read_write_loop);
                    } else {
                        error("\nReceive error\n");
                    }

                    FD_CLR(i, &readfds);
                } else {
                    //We have some data from the connection
                                            //... Manipulate the buffer
                    //Handle the message
                }
            }
            }

            if(FD_ISSET(i, &tempwritefds)) {
                                .....
                FD_CLR(i, &writefds);
            }

            //Timer checking
            if(connections[i].active) {
                gettimeofday(&TimeNow, NULL);
                timeval_diff(&Interval, &TimeNow, &connections[i].TimeConnected);
                printf("*_*_*__*_*_*__*_*_*_*_* difference is %ld seconds, %ld microseconds\n",
                         Interval.tv_sec,
                         Interval.tv_usec
                        );
                if(Interval.tv_sec >= QUIT_TIME) {
                    printf("Timer elapsed!!\n");
                }
            }

        }
    }

    /* clean up */
    printf("Terminating server.\n");
    close(parentfd);
    return 0;
}

void InitializeDataStructures(int i) {

    clients[i].active = YES;
    clients[i].fd = i;
    //Initialize other members of the structure
}

long long timeval_diff(struct timeval *difference, timeval *end_time, struct timeval *start_time) {
      struct timeval temp_diff;

      if(difference==NULL)
        difference=&temp_diff;

      difference->tv_sec =end_time->tv_sec -start_time->tv_sec ;
      difference->tv_usec=end_time->tv_usec-start_time->tv_usec;

      while(difference->tv_usec<0)
      {
        difference->tv_usec+=1000000;
        difference->tv_sec -=1;
      }

      return 1000000LL*difference->tv_sec + difference->tv_usec;

    }

Solution

  • Take a look at your select loop parameters, they look fishy to me. Mainly in that you are calling select on tempreadfd and tempwritefd, but then when you call FD_ISSET, you pass it readfd and writefd. before calling select, you are using FD_SET to set all the fd's you are interested in. Since these variables aren't being sent to select, the fds that haven't triggered aren't getting masked. Therefore, you are getting 'activity' detected on all your descriptors. There really isn't any activity on that accept descriptor, so it blocks until a new client connects.

    Thats my guess at least.