[Freeciv-Dev] Re: (PR#4757) server_crash: pending_seen is off

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

(Reposted to RT.)

I'm 90% sure that the pending_seen bug was caused by the F_GAMELOSS 
code, which caused barbarian players who died because they lost their 
barbarian leader to never be properly killed off (PR#5097).  This 
resulted in extra units and cities floating around that would be 
"inherited" by the next barbarian player to be created.  This bug was 
fixed a month ago (8/19).

The two savegames attached to RT don't reproduce the bug - the bug is 
embedded in the savegame since the "dead" player has units and cities. 
We can't tell for certain where these units/cities came from, but the 
GAMELOSS scenario is the only possible one I can imagine.

Has anyone gotten a _new_ instance of this bug in the last month?  If 
so, we're back to square one: we need a savegame.  We'll probably need 
multiple savegames since the most recent one may not reproduce the bug 
itself.

The attached patch correctly loads the buggy savegames, and includes a 
sanity check to detect such errors in future.  I suspect this is all 
that's needed to address this bug, but only time (and the sanity checks) 
will tell.

jason

Index: server/sanitycheck.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/sanitycheck.c,v
retrieving revision 1.31
diff -u -r1.31 sanitycheck.c
--- server/sanitycheck.c        2003/08/01 15:58:08     1.31
+++ server/sanitycheck.c        2003/09/18 14:19:53
@@ -270,6 +270,8 @@
 **************************************************************************/
 static void check_players(void)
 {
+  int player_no;
+
   players_iterate(pplayer) {
     int found_palace = 0;
 
@@ -288,6 +290,20 @@
               == pplayer2->diplstates[pplayer->player_no].turns_left);
     } players_iterate_end;
   } players_iterate_end;
+
+  /* Sanity checks on living and dead players. */
+  for (player_no = 0; player_no < ARRAY_SIZE(game.players); player_no++) {
+    struct player *pplayer = &game.players[player_no];
+
+    if (!pplayer->is_alive) {
+      /* Dead players' units and cities are disbanded in kill_player(). */
+      assert(unit_list_size(&pplayer->units) == 0);
+      assert(city_list_size(&pplayer->cities) == 0);
+    }
+
+    /* Dying player should have been killed already. */
+    assert(!pplayer->is_dying);
+  }
 }
 
 #endif /* !NDEBUG */
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.135
diff -u -r1.135 savegame.c
--- server/savegame.c   2003/09/09 15:49:07     1.135
+++ server/savegame.c   2003/09/18 14:19:53
@@ -781,7 +781,10 @@
 
   city_list_init(&plr->cities);
   ncities=secfile_lookup_int(file, "player%d.ncities", plrno);
-  
+  if (!plr->is_alive && ncities > 0) {
+    ncities = 0; /* Some old savegames may be buggy. */
+  }
+
   for (i = 0; i < ncities; i++) { /* read the cities */
     struct city *pcity;
 
@@ -959,7 +962,10 @@
 
   unit_list_init(&plr->units);
   nunits=secfile_lookup_int(file, "player%d.nunits", plrno);
-  
+  if (!plr->is_alive && nunits > 0) {
+    nunits = 0; /* Some old savegames may be buggy. */
+  }
+
   for(i=0; i<nunits; i++) { /* read the units */
     struct unit *punit;
     struct city *pcity;