1
votes

As a school project, I'm implementing a IRC server and I've been stuck on this one problem for the day. My server use select to choose which client is sending data, it then read one command from this user (commands are \r\n separated), parse it and execute it before passing to the next user. A user can send multiple command at once like so :

"command1\r\ncommand2\r\n"

If this happen, i want the first command to be executed and the second to stay in the stream so that it can be read at the next select() call. (this way, each user only execute one comamnd per "turn"). To do this, I have a FILE *stream per client that stay open as long as it is connected.

This work perfectly (if I send the double comamnd specified above, the two commands are executed one after the other).

The problem is after that after that, select() continue to tell me that there is something to read in the socket (FD_ISSET of the fd return 1), so my receive function is called for that fd and the getline() in it fail (without setting errno) and it loop like this forever.

I don't understand why the select still consider that there is something to read in the socket and why the getline is failing instead of blocking.

Any idea ?

edit: I'm not allowed to use non blocking socket or fork() for this project

The "main" loop:

  while (g_run_server)                                                                
    { 
      fd_max = g_socket_fd;                                                                                                                                                
      FD_ZERO(fds);                                                                       
      FD_SET(g_socket_fd, fds);                                                           
      tmp = users;                                                                        
      while (tmp)                                                                         
        {                                                                                 
          FD_SET(tmp->fd, fds);                                                           
          fd_max = (tmp->fd > fd_max ? tmp->fd : fd_max);                              
          tmp = tmp->next;                                                                
        }
      if (select(fd_max + 1, &fds, NULL, NULL, NULL) < 0)                             
         break;                                                                 
      if (FD_ISSET(g_socket_fd, &fds))                                                
        accept_new_user(&hdl, &users);                                                
      handle_clients(&hdl, &fds);                                                     
    } 

The functions to handle and read client input :

static bool recv_and_execute(t_handle *hdl)
{
  char      *raw;
  size_t    len;

  len = 0;
  raw = NULL;
  if (!hdl->sender->stream &&
      (hdl->sender->stream = fdopen(dup(hdl->sender->fd), "r")) == NULL)
    return (false);
  if (getline(&raw, &len, hdl->sender->stream) != -1)
    {
      printf("Received \"%s\"\n", raw);
      parse_cmd(hdl, raw);
      exec_cmd(hdl);
    }
  else
    printf("Getline failed %s\n", strerror(errno));
  free(raw);
  return (true);
}

int         handle_clients(t_handle *hdl, fd_set *fds)
{
  t_user        *tmp;

  tmp = *hdl->users;
  while (tmp)
    {
      if (FD_ISSET(tmp->fd, fds))
        {
          printf("fd %d is ready to be read\n", tmp->fd);
          hdl->sender = tmp;
          recv_and_execute(hdl);
          FD_CLR(tmp->fd, fds);
      tmp = tmp->next;
      if (hdl->sender->status == DEAD)
        del_user(hdl->users, hdl->sender);
        }
      else
        tmp = tmp->next;
    }
  return (0);
}

And this is the output when I connect one client and send "USER foo\r\nUSER no bo dy :wa\r\n" :

fd 4 is ready to be read
Received "NICK foo
"
[DEBUG] Executing "NICK" with params "foo" "(null)" "(null)" "(null)"
[INFO] Nickname successfully changed.
fd 4 is ready to be read
Received "USER no bo dy :wa
"
[DEBUG] Executing "USER" with params "no" "bo" "dy" ":wa"
[INFO] User registered.
fd 4 is ready to be read
Getline failed Success
fd 4 is ready to be read
Getline failed Success
fd 4 is ready to be read
Getline failed Success
fd 4 is ready to be read
Getline failed Success
fd 4 is ready to be read
Getline failed Success
continue like this....

Edit : I edited my receive function based on the comment of alk:

static bool     recv_and_execute(t_handle *hdl)                                               
{                                                                                             
  char          *raw;                                                                         
  size_t        len;                                                                          
  ssize_t       nread;                                                                        

  len = 0;                                                                                    
  raw = NULL;                                                                                 
  if (!hdl->sender->stream &&                                                                 
      (hdl->sender->stream = fdopen(dup(hdl->sender->fd), "r")) == NULL)                      
    return (false);                                                                           
  errno = 0;                                                                                  
  if ((nread = getline(&raw, &len, hdl->sender->stream)) > 0)                                 
    {                                                                                         
      printf("Received \"%s\"\n", raw);                                                       
      parse_cmd(hdl, raw);                                                                    
      exec_cmd(hdl);                                                                          
    }                                                                                         
  else if (errno != 0)                                                                        
    printf("getline failed %s\n", strerror(errno));                                           
  else {                                                                                      
    printf("EOF reached\n");                                                                  
    fclose(hdl->sender->stream);                                                              
    hdl->sender->stream = NULL;                                                               
  }                                                                                           
  printf("nread = %zd\n", nread);                                                             
  free(raw);                                                                                  
  return (true);                                                                              
}

So this time, when EOF is reach (getline return -1), I close the stream and set it to NULL to be reopened the next time select find data on the socket fd. But even when I close the stream, select still detect that there is data available and the inifinite loop continue :/

fd 4 is ready to be read                                                                       
Received "NICK foo^M                                                                           
"                                                                                              
nread = 10                                                                                     
fd 4 is ready to be read                                                                       
Received "USER no bo dy :wa^M                                                                  
"                                                                                              
nread = 19                                                             
fd 4 is ready to be read                                                                       
EOF reached                                                                                    
nread = -1                                                                                     
fd 4 is ready to be read                                                                       
EOF reached                                                                                    
nread = -1                                                                                     
fd 4 is ready to be read                                                                       
EOF reached                                                                                    
nread = -1
and so on... 
1
The order in which you call select and then zero/set the fds set seems like it should be the opposite. - Some programmer dude
*fd_max = g_socket_fd; what is this supposed to do? Does it compile? well, fd_max could be a pointer (you didnt show us), but in that case select(fd_max + 1,... would have been invalid - joop
@Someprogrammerdude Ooops, seems like I can't even copy and paste to stackoverflow properly, I edited my post - Theo Champion
g_socket_fd is the fd off the server socket (global to close it in signal_handler) - Theo Champion
Likely you are "reading" the end-of-file on the fd. - John Hascall

1 Answers

0
votes

I'm pretty sure you're using the select wrong. I show you a simple code example on how to use it (I don't handle many errors) and you can edit it as you need.

/*
 * If you read at man select bugs you can see
 * that select could return that someone is 
 * ready but it isn't true
 */
    int fd_c;
    fd_set rdset;
    fd_set set; /*That's the mask you'll use when new requests arrive*/
    FD_ZERO(&set);  /*Clears the mask*/
    FD_SET(g_socket_fd,&set); /*Set the listening socket as ready*/
    while(g_run_server){
        /*
        * YOU MUST INITIALIZATE IT EVERY LOOP
        * read @ man select
        */
        rdset = set;
        if(select((fd_num_max+1),(&rdset),NULL,NULL,NULL) < 0){
            perror("Failed on select\n");
            exit(EXIT_FAILURE);
        }
        /*You go through the ready clients in the rdset*/
        for(fd=0;fd<=fd_num_max;fd++) {
            if(FD_ISSET(fd,&rdset)) { /*If the bit is set*/
                if(fd == fd_skt) { /*If it's a new client*/
                    /*You can handle the new client here or delegete it to someone*/
                    fd_c=accept(fd_skt,NULL,0); /*File descriptor of new client*/
                    FD_SET(fd_c,&set); 
                    if(fd_c > fd_num_max) fd_num_max = fd_c;
                }else { /*If it's a request from an existing client*/
                    /*You can handle the new request here or delegete it to someone*/
                    FD_SET(fd,&set);
                }
            }
        }
    }

You should also modify static bool recv_and_execute(t_handle *hdl) that way:

errno = 0;
if ((nread = getline(&raw, &len, hdl->sender->stream)) != -1){                                                                                         
      printf("Received \"%s\"\n", raw);                                                       
      parse_cmd(hdl, raw);                                                                    
      exec_cmd(hdl);                                                                          

}else{
    if( errno == 0){
        printf("EOF reached\n");                                                                  
        fclose(hdl->sender->stream);                                                              
        hdl->sender->stream = NULL;
    }else{
        printf("getline failed %s\n", strerror(errno)); 
        exit(EXIT_FAILURE); /*You must handle it in some way, exiting or doing something*/
    }
}