[Freeciv-Dev] (PR#3664) Server doesn't react while send_all_info(&pconn->self);

["Vasco Alexandre da Silva Costa" <vasc@xxxxxxxxxxxxxx>]

[jdorje - Tue Mar 11 10:52:26 2003]:

> I suspect tcptimeout is not checked within flush_packets.

It is. Just not in the way you probably think it is:

/****************************************************************************
  Attempt to flush all information in the send buffers for upto 'netwait'
  seconds.
*****************************************************************************/
void flush_packets(void)
{
  int i;
  int max_desc;
  fd_set writefs, exceptfs;
  struct timeval tv;
  time_t start;

  (void) time(&start);

  for(;;) {
    tv.tv_sec=(game.netwait - (time(NULL) - start));
    tv.tv_usec=0;

    if (tv.tv_sec < 0)
      return;

    MY_FD_ZERO(&writefs);
    MY_FD_ZERO(&exceptfs);
    max_desc=-1;

    for(i=0; i<MAX_NUM_CONNECTIONS; i++) {
      struct connection *pconn = &connections[i];
      if(pconn->used && pconn->send_buffer->ndata > 0) {
        FD_SET(pconn->sock, &writefs);
        FD_SET(pconn->sock, &exceptfs);
        max_desc=MAX(pconn->sock, max_desc);
      }
    }

    if (max_desc == -1) {
      return;
    }

    if(select(max_desc+1, NULL, &writefs, &exceptfs, &tv)<=0) {
      return;
    }

    for(i=0; i<MAX_NUM_CONNECTIONS; i++) {   /* check for freaky players */
      struct connection *pconn = &connections[i];
      if(pconn->used) {
        if(FD_ISSET(pconn->sock, &exceptfs)) {
          freelog(LOG_NORMAL, "cut connection %s due to exception data",
                  conn_description(pconn));
          close_socket_callback(pconn);
        } else {
          if(pconn->send_buffer && pconn->send_buffer->ndata > 0) {
            if(FD_ISSET(pconn->sock, &writefs)) {
              flush_connection_send_buffer_all(pconn);
            } else {
              if(game.tcptimeout != 0 && pconn->last_write != 0
                 && (time(NULL)>pconn->last_write + game.tcptimeout)) {
                freelog(LOG_NORMAL, "cut connection %s due to lagging
player",
                        conn_description(pconn));
                close_socket_callback(pconn);
              }
            }
          }
        }
      }
    }
  }
}


> In any case if flush_packets waits game.netwait seconds, then it should
> not be called repeatedly without processing incoming packets in
> between
> (this is the point of the variable, right?).  I would suggest either

Fact is the game has other places where it does not process incoming
packets either. e.g. during the AI processing phase.

> that it be removed from the send_all_known_tiles loop (which may
> result
> in immediately cutting all players if the connection buffers are too
> small, I think - this is true already but the needed buffer would be
> ~80x larger).  Alternately we can just set netwait to 0 by default (or
> possibly just do this for pubserver).

The connection buffers currently have a 512 KB size limit. This is
reasonable considering that MAX_NUM_PLAYERS is 30 and pubserver runs 
16 civservers at any given time. IMHO this can be doubled at most. The
machine needs memory for more things than civserver network buffers. :-)

Setting netwait to 0 effectively disables flush_packets. So if you don't
like it that is the way to go I guess.

If you do this extend tcptimeout appropriately to get less disconnects.

> I think the server should never force the game to wait for network
> reasons.  As Raimar says, network usage tends to be clumped, so in
> most
> cases this waiting is unnecessary.  Players may see performance
> decreases (by the time their client has caught up, their opponents
> have
> finished the battle and killed all their units), but in a game with
> sufficiently high timeout and large enough connection buffers nobody
> should be unnecessarily disconnected.  (It is possible this can be
> entirely accomplished just by setting netwait to 0.)

If we reduce the size of the data being sent I believe flush_packets is
unnecessary since the buffers will take up the slack.

I also considered at a time using a separate thread just for flushing
the buffers since this would reduce the possibility of the buffers
getting full. In the interests of portability I didn't pursue this
further. However I still think it would be a good idea.

> A more complicated solution to the problem would extend the (turn)
> timeout when a player was lagging.  So if you play with a 5s timeout
> and
> one player hasn't fully caught up by this time, the turn continues
> until
> they do (or they are cut because of pingtimeout or tcptimeout).  The
> implemention would probably be error-prone and potentially abusable,
> though.
> 
> All of this may require making the connection buffers larger.

Thank God for VM but don't overdo it. :-)