Properly writing to a nonblocking socket in C++

Question

I'm having a strange problem while attempting to transform a blocking socket server into a nonblocking one. Though the message was only received once when being sent with blocking sockets, using nonblocking sockets the message seems to be received an infinite number of times. Here is the code that was changed:

return ::write(client, message, size);

to

// Nonblocking socket code
int total_sent = 0, result = -1;
while( total_sent < size ) {
   // Create a temporary set of flags for use with the select function
   fd_set working_set;
   memcpy(&working_set, &master_set, sizeof(master_set));

   // Check if data is available for the socket - wait 1 second for timeout
   timeout.tv_sec = 1;
   timeout.tv_usec = 0;
   result = select(client + 1, NULL, &working_set, NULL, &timeout);

    // We are able to write - do so
   result = ::write(client, &message[total_sent], (size - total_sent));
   if (result == -1) {
      std::cerr << "An error has occured while writing to the server."
              << std::endl;
      return result;
   }
   total_sent += result;
}

return 0;

EDIT: The initialization of the master set looks like this:

// Private member variables in header file
fd_set master_set;
int sock;

...

// Creation of socket in class constructor
sock = ::socket(PF_INET, socket_type, 0);

// Makes the socket nonblocking
fcntl(sock,F_GETFL,0);

FD_ZERO(&master_set);
FD_SET(sock, &master_set);

...

// And then when accept is called on the socket
result = ::accept(sock, NULL, NULL);
if (result > 0) {
   // A connection was made with a client - change the master file
   // descriptor to note that
   FD_SET(result, &master_set);
}

I have confirmed that in both cases, the code is only being called once for the offending message. Also, the client side code hasn't changed at all - does anyone have any recommendations?

Answer 1

I do not believe that this code is really called only once in the "non blocking" version (quotes because it is not really non-blocking yet as Maister pointed out, look here), check again. If the blocking and non blocking versions are consistent, the non blocking version should return total_sent (or size). With return 0 instead caller is likely to believe nothing was sent. Which would cause infinite sending... is it not what's happening ?

Also your "non blocking" code is quite strange. You seem to use select to make it blocking anyway... Ok, with a timeout of 1s, but why don't you make it really non blocking ? ie: remove all the select stuff and test for error case in write() with errno being EWOULDBLOCK. select or poll are for multiplexing.

Also you should check errors for select and use FD_ISSET to check if socket is really ready. What if the 1 s timeout really happen ? Or if select is stopped by some interruption ? And if an error occurs in write, you should also write which error, that is much more useful than your generic message. But I guess this part of code is still far from finished.

As far as I understand your code it should probably look somewhat like that (if the code is running in an unique thread or threaded, or forking when accepting a connection would change details):

// Creation of socket in class constructor
sock = ::socket(PF_INET, socket_type, 0);
fcntl(sock, F_SETFL, O_NONBLOCK);

// And then when accept is called on the socket
result = ::accept(sock, NULL, NULL);
if (result > 0) {
   // A connection was made with a client
   client = result;
   fcntl(client, F_SETFL, O_NONBLOCK);
}

// Nonblocking socket code
result = ::write(client, &message[total_sent], (size - total_sent));
if (result == -1) {
  if (errno == EWOULDBLOCK){
      return 0;
  }
  std::cerr << "An error has occured while writing to the server."
          << std::endl;
  return result;
}
return size;

Answer 2

fcntl(sock,F_GETFL,0);

How does that make the socket non-blocking?

fcntl(sock, F_SETFL, O_NONBLOCK);

Also, you are not checking if you can actually write to the socket non-blocking style with

FD_ISSET(client, &working_set);