[Freeciv-Dev] Re: (PR#6819) server crashes when loaded trireme is lost

["Raimar Falke" <i-freeciv-lists@xxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=6819 >

On Mon, Nov 10, 2003 at 07:09:54AM -0800, Genevieve Gracian wrote:
> 
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=6819 >
> 
> 
> In recent head cvs, when a loaded trireme is lost in high sea, 
> the server crashes. It seems that it only occurs when the unit in 
> the trireme has been built by a city (no problem with explorers of 
> the begining even if they've been assigned to a homecity).

A nice crash.

stack trace:

#0  0x080c7ad6 in unit_type (punit=0x10) at unittype.c:96
#1  0x0805d4f2 in unit_restore_hitpoints (pplayer=0x10, punit=0x10)
    at unittools.c:521
#2  0x0805cfe3 in player_restore_units (pplayer=0x81fe808) at unittools.c:424
#3  0x0804e6b9 in end_turn () at srv_main.c:504
#4  0x0804ff0e in main_loop () at srv_main.c:1540
#5  0x080500ca in srv_main () at srv_main.c:1624
#6  0x0804a13e in main (argc=136936160, argv=0xbffff194) at civserver.c:154
#7  0x400ce1c4 in __libc_start_main () from /lib/libc.so.6


Adding debug output (see patch) you get
2: player=0x81fe868
2:   unit=0x82ee840
2:     unit2 start
2:     unit2=0x82ee840
2:     unit2=0x82ee8d0
2:     unit2=0x82ee960
2:     unit2=0x82ee9f0
2:     unit2 end
2:   unit=0x10
2:     unit2 start
2:     unit2=0x82ee960
2:     unit2=0x82ee9f0
2:     unit2 end
Speicherzugriffsfehler (core dumped)

So you see that the list is ok but the iterator wrong.

I strongly think this is connected to 

revision 1.258
date: 2003/10/08 16:56:07;  author: jdorje;  state: Exp;  lines: +12 -27
Remove the wipe_unit_safe function, as wipe_unit is inherently safe.
wipe_unit_spec_safe remains, but it doesn't need the iterator parameter
any more.

Patch by Per as PR#6408.

but Jason doesn't know why "wipe_unit is inherently safe". I have
searched the changes of unittools.c and genlist.c but none indicated
that something was changed in them that will make removing in an item
in a list safe while you iterate over it.

So Per, Jason can you shed more light on this?

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  "brand memory are for windows users that think their stability
   problems come from the memory"
    -- bomek in #freeciv

Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.263
diff -u -u -r1.263 unittools.c
--- server/unittools.c  2003/11/04 10:13:28     1.263
+++ server/unittools.c  2003/11/10 19:54:38
@@ -412,11 +412,19 @@
 ****************************************************************************/
 void player_restore_units(struct player *pplayer)
 {
+    freelog(LOG_NORMAL,"player=%p",pplayer);
   /* 1) get Leonardo out of the way first: */
   if (player_owns_active_wonder(pplayer, B_LEONARDO))
     handle_leonardo(pplayer);
 
   unit_list_iterate(pplayer->units, punit) {
+    freelog(LOG_NORMAL,"  unit=%p",punit);
+
+    freelog(LOG_NORMAL,"    unit2 start");
+    unit_list_iterate(pplayer->units, punit2) {
+       freelog(LOG_NORMAL,"    unit2=%p",punit2);
+    }unit_list_iterate_end;
+    freelog(LOG_NORMAL,"    unit2 end");
 
     /* 2) Modify unit hitpoints. Helicopters can even lose them. */
     unit_restore_hitpoints(pplayer, punit);