crash in map object saver

Bug #670980 reported by Timowi
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Fix Released
High
Unassigned

Bug Description

I just found another rare crash. Widelands crashes in Map_Map_Object_Saver::get_object_record() because it is called from Cmd_EnemyFlagAction::Write() with a &obj = NULL. I think this happens in the following case:
1. Cmd_EnemyFlagAction is enqueued
2. MapObject (Flag?) is destroyed
3. Game is saved before the Command get executed.

Maybe this can happen for other commands too.

gdb output:
 Game: Writing Map Data done!
Game: Writing Player Economies Info ... done
Game: Writing Command Queue Data ... nwritten: 1764

Program received signal SIGSEGV, Segmentation fault.
Widelands::Map_Map_Object_Saver::get_object_record (this=0x2f0e4f38, obj=...) at /home/timo/widelands/trunk/src/map_io/widelands_map_map_object_saver.cc:60
60 rec.description = obj.type_name();
(gdb) bt full 2
#0 Widelands::Map_Map_Object_Saver::get_object_record (this=0x2f0e4f38, obj=...) at /home/timo/widelands/trunk/src/map_io/widelands_map_map_object_saver.cc:60
        rec = {description = {static npos = 4294967295, _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x2dbee1b0 "H\376I\b\333\071\361\016\004"}}, fileserial = 4156030927,
          registered = 148, saved = 148}
#1 0x08358d98 in Widelands::Map_Map_Object_Saver::get_object_file_index (this=0x2f0e4f38, obj=...) at /home/timo/widelands/trunk/src/map_io/widelands_map_map_object_saver.cc:123
No locals.
(More stack frames follow...)
(gdb) bt
#0 Widelands::Map_Map_Object_Saver::get_object_record (this=0x2f0e4f38, obj=...) at /home/timo/widelands/trunk/src/map_io/widelands_map_map_object_saver.cc:60
#1 0x08358d98 in Widelands::Map_Map_Object_Saver::get_object_file_index (this=0x2f0e4f38, obj=...) at /home/timo/widelands/trunk/src/map_io/widelands_map_map_object_saver.cc:123
#2 0x081a34d7 in Widelands::Cmd_EnemyFlagAction::Write (this=0x2dbee1b0, fw=..., egbase=..., mos=...) at /home/timo/widelands/trunk/src/logic/playercommand.cc:1246
#3 0x08324f65 in Widelands::Game_Cmd_Queue_Data_Packet::Write (this=0xffff9534, fs=..., game=..., os=0x2f0e4f38) at /home/timo/widelands/trunk/src/game_io/game_cmd_queue_data_packet.cc:123
#4 0x0832115d in Widelands::Game_Saver::save (this=0xffff9580) at /home/timo/widelands/trunk/src/game_io/game_saver.cc:70
#5 0x081509d0 in SaveHandler::save_game (this=0xffff9a20, game=..., complete_filename=..., error=0x85722a8) at /home/timo/widelands/trunk/src/save_handler.cc:155
#6 0x08150c2b in SaveHandler::think (this=0xffff9a20, game=..., realtime=10762433) at /home/timo/widelands/trunk/src/save_handler.cc:69
#7 0x0819377d in Widelands::Game::think (this=0xffff987c) at /home/timo/widelands/trunk/src/logic/game.cc:570
#8 0x082c2b7e in Interactive_Base::think (this=0x1883d878) at /home/timo/widelands/trunk/src/wui/interactive_base.cc:332
#9 0x0817ae28 in UI::Panel::do_think (this=0x1883d878) at /home/timo/widelands/trunk/src/ui_basic/panel.cc:542
#10 0x0817ca71 in UI::Panel::run (this=0x1883d878) at /home/timo/widelands/trunk/src/ui_basic/panel.cc:174
#11 0x081980a4 in Widelands::Game::run (this=0xffff987c, loader_ui=..., start_game_type=Widelands::Game::NewNonScenario) at /home/timo/widelands/trunk/src/logic/game.cc:535
#12 0x081591a0 in NetHost::run (this=0xffffb248, autorun=false) at /home/timo/widelands/trunk/src/network/nethost.cc:653
#13 0x0812d52b in WLApplication::mainmenu_multiplayer (this=0x858d228) at /home/timo/widelands/trunk/src/wlapplication.cc:1695
#14 0x081331ce in WLApplication::mainmenu (this=0x858d228) at /home/timo/widelands/trunk/src/wlapplication.cc:1480
#15 0x08133745 in WLApplication::run (this=0x858d228) at /home/timo/widelands/trunk/src/wlapplication.cc:419
#16 0x08129cb1 in main (argc=1, argv=0xffffd294) at /home/timo/widelands/trunk/src/main.cc:48
(gdb) bt full 4
#0 Widelands::Map_Map_Object_Saver::get_object_record (this=0x2f0e4f38, obj=...) at /home/timo/widelands/trunk/src/map_io/widelands_map_map_object_saver.cc:60
        rec = {description = {static npos = 4294967295, _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x2dbee1b0 "H\376I\b\333\071\361\016\004"}}, fileserial = 4156030927,
          registered = 148, saved = 148}
#1 0x08358d98 in Widelands::Map_Map_Object_Saver::get_object_file_index (this=0x2f0e4f38, obj=...) at /home/timo/widelands/trunk/src/map_io/widelands_map_map_object_saver.cc:123
No locals.
#2 0x081a34d7 in Widelands::Cmd_EnemyFlagAction::Write (this=0x2dbee1b0, fw=..., egbase=..., mos=...) at /home/timo/widelands/trunk/src/logic/playercommand.cc:1246
        obj = <value optimized out>
#3 0x08324f65 in Widelands::Game_Cmd_Queue_Data_Packet::Write (this=0xffff9534, fs=..., game=..., os=0x2f0e4f38) at /home/timo/widelands/trunk/src/game_io/game_cmd_queue_data_packet.cc:123
        cmd = <value optimized out>
        fw = {<Widelands::StreamWrite> = {<StreamWrite> = {_vptr.StreamWrite = 0x84c5d38}, <No data fields>}, data = 0x2fb158c8 "\002", filelength = 177547, maxsize = 180224, filepos = {pos = 177547}}
(More stack frames follow...)
(gdb) print &obj
$1 = (const Widelands::Map_Object *) 0x0

Tags: crash savegame
Revision history for this message
Timowi (timo-wingender) wrote :
Revision history for this message
SirVer (sirver) wrote :

The approach to not write the package if the object has vanished seems like a good one to me. Why didn't you apply this patch?

Revision history for this message
Timowi (timo-wingender) wrote :

I am unable to reproduce or test this. It was just a quick Idea to prevent the crash. I hope to get comments about how to fix it here. At the first glance this patch seems to fix the problem. After thinking a bit more about this (and searching other Problems in the code) I think this will only fix the crash but scramble the savegame.

I am not sure with this but a far as I know Widelands::Cmd_EnemyFlagAction::Write only writes Cmd_EnemyFlagAction related parts of the command queue. At this time Widelands::Game_Cmd_Queue_Data_Packet::Write has already written Queue data packet related parts (id, gametime, category) to the savegame. So simply returning and write nothing will write a incomplete Game_Cmd_Queue_Data_Packet to the file. This will break the savegame.

If I remember right Widelands::Game_Cmd_Queue_Data_Packet::Write creates a new StreamWrite for the Packet. Then one solution would be to throw a "vanished object exception", catch this in Widelands::Game_Cmd_Queue_Data_Packet::Write and not write the packet at all in this case.

Revision history for this message
SirVer (sirver) wrote :

you are right, that would leave a halfway written packet in the save game.

Revision history for this message
Nasenbaer (nasenbaer) wrote :

why not writing the Command to the savegame with a dummy serial or something like that - I wonder what happens at game time, if such a command is executed after the flag was removed - obviously it is just ignored and deleted, else there surely were some bug reports about that topic.

So writing the command anyways, but with an invalid serial?

Revision history for this message
Nicolai Hähnle (nha) wrote :

Fixed in r5874, along the lines Nasenbaer suggested.

Changed in widelands:
status: Confirmed → Fix Committed
Revision history for this message
SirVer (sirver) wrote :

Released in build16-rc1

Changed in widelands:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.