0007146: Saved with a dead monster in a transit list, now crashes on load

Notes
(0023033) neil (administrator) 2013-06-03 16:11 edited on: 2013-06-03 16:12	The game seems to restore, then crashes during the initial save (the following dump is from loading it in trunk). #0 0xf7748430 in __kernel_vsyscall () #1 0xf7481610 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:42 0000002 0x0842c34b in _BreakStrToDebugger ( mesg=0xffd9e358 "ASSERT(!invalid_monster_type(f.mons.type)) in 'tags.cc' at line 1543 faile d.", assert=true) at dbg-asrt.cc:776 0000003 0x0842c4a2 in AssertFailed ( expr=0x8b17218 "!invalid_monster_type(f.mons.type)", file=0x8b17113 "tags.cc", line=1543, text=0x8b17100 "") at dbg-asrt.cc:818 0000004 0x088dcdfb in marshall_follower (th=..., f=...) at tags.cc:1543 0000005 0x088dd046 in marshall_follower_list (th=..., mlist=std::list) at tags.cc:1581 0000006 0x088e9937 in marshallMap<level_id, std::list<follower, std::allocator<follower> > ; > (th=..., data=std::map with 1 elements, key_marshall=0x88d9b94 <marshall_level_id(writer&, level_id const&)>, value_marshall=0x88dcff4 <marshall_follower_list(writer&, m_transit_list const&)>) at tags.cc:506 0000007 0x088ddcc9 in tag_construct_lost_monsters (th=...) at tags.cc:1787 0000008 0x088da7da in tag_write (tagID=TAG_YOU, outf=...) at tags.cc:965 0000009 0x084fb516 in _write_tagged_chunk (chunkname="you", tag=TAG_YOU) at files.cc:816 0000010 0x084fdf90 in _save_game_base () at files.cc:1575 0000011 0x084fe483 in save_game (leave_game=false, farewellmsg=0x0) at files.cc:1618 0000012 0x084fe54b in save_game_state () at files.cc:1638 0000013 0x086337e6 in _launch_game () at main.cc:468 0000014 0x086333da in _launch_game_loop () at main.cc:384 0000015 0x08633251 in main (argc=1, argv=0xffd9efd4) at main.cc:342

(0023036) wagnor (reporter) 2013-06-03 19:44	I've tried creating air elementals using air elemental fan at abyss:5, then going to vestibule (all using wizmode), then saving and loading - but everything seems fine. I don't know what circumstances led to a faulty savegame.

(0023037) neil (administrator) 2013-06-03 21:16	It appears that this can (only?) happen if a dead monster ends up in a transiting monster list. The first save converted the dead monster to MONS_NO_MONSTER and succeeded; subsequent saves (including the save that happens immediately after loading a game) fail because of the MONS_NO_MONSTER.

(0023693) neil (administrator) 2013-08-09 21:35 edited on: 2013-08-09 22:28	Another save with the same problem: http://crawl.lantea.net/saves/foonesh-crawl-git-56e3d78c6f-130809-1613.tar.bz2 [^] One example crash log: http://crawl.lantea.net/crawl/morgue/foonesh/crash-foonesh-20130809-195522.txt [^] The dead monster here was in transit to Abyss:1. The player was not using distortion or Lugonu; the only allies were evoker elementals.

(0023795) blackcustard (reporter) 2013-08-20 04:59 edited on: 2013-08-20 22:15	1. Wagnor's game - Naga Wizard of Makhleb - 0.12.1 - visited Abyss https://crawl.develz.org/mantis/file_download.php?file_id=4685&type=bug [^] 2. foonesh's game - Gargoyle Gladitor of Ashenzari - 0.13 trunk revision 56e3d78c6fd4ac2f6bca401fbc4b825066c3eb98 - visited Abyss http://crawl.lantea.net/crawl/morgue/foonesh/crash-foonesh-20130809-195522.txt [^] These crashes are actually a different manifestation of an mgrd corruption which \|amethyst said kilobyte has been observing in his stress tests, where a dead starcursed mass somehow gets recorded in the mgrd. A dead monster linked into the mgrd is usually harmless, and for this reason debug_scan in dbg-mon just prints an error message rather than ASSERTing. However, _abyss_wipe_square_at in abyss.cc pulls monsters directly from the mgrd and sometimes places them in the transit list WITHOUT CHECKING WHETHER OR NOT THEY ARE ALIVE. So the dead monsters that Wagnor and foonesh ended up with in their transit list, which were transformed on game-load into M_NO_MONSTERS, were originally dead starcursed masses that kilobyte's bug had somehow introduced into the mgrd. There is no crash when a dead monster is added to the transiting list, no crash when a dead monster in the transiting list is placed (though this will corrupt the mgrd), and no crash when a dead monster in the transiting list is saved. However, the save will convert the dead monster into M_NO_MONSTER. There IS a crash when an M_NO_MONSTER in the transiting list is saved. So to get Wagnor and foonesh's crash you need to: 1. Trigger kilobyte's starcursed mass thing (or cast cure poison with my patch -- see below) 2. Get the Abyss to add the dead starcursed mass (or dead Rupert) to the transiting monsters list 3. Save and stop playing before the dead starcursed mass (or dead Rupert) gets placed The save you created in step three will be corrupt. It will load successfully, but Crawl will immediately turn around and try to save it again, and then it will crash. I've attached a patch which is NOT ready to apply. It will show you what I'm talking about. I modified cure poison to corrupt the mgrd for testing purposes. I also added some asserts, and modified the unmarshalling code. If you load a corrupted save with this patch, the bad monster in the transit list will be removed. (So Wagnor, if you are desperate to resume that game then there you go :p.) EDIT: I used to have two completely unrelated crashes listed here. They came from a bug fixed by 885b5f1. I dug the crash logs up by querying Sequell for "ASSERT(!invalid_monster_type" and thought they were related, despite all the evidence that they weren't. (They were both Beogh followers, they were both around March 10 2013, they were both in the same small range of commits). Pretty stupid, but I guess that happens sometimes. Just gotta chase down whatever thing is corrupting the mgrd with dead monsters in the first place then.

(0024600) neil (administrator) 2013-12-05 00:26	The starcursed mass crash was fixed back in September. I also just fixed another problem that could lead to dead/invalid monsters in the list (0.14-a0-1306-gf0ae609). It might make sense to apply this patch, minus the Cure Poison thing of course. Probably the warning message in _abyss_wipe_square_at should become an assert instead. Otherwise, I think the general approach blackcustard takes of "assert on marshall, fix up on unmarshall" is a good one to catch bugs while not breaking saves.

(0024601) neil (administrator) 2013-12-05 00:58	Applied in trunk (0.14-a0-1307-gd1885e8) with several changes: * Remove the testing hack in Cure Poison. * Change the warning message in _abyss_wipe_square_at into an ASSERT, since we have fixed the known bugs that could leave dead starcursed masses in the grid. * Use MSGCH_ERROR rather than the in-character MSGCH_WARN for the warning message in unmarshall_follower_list. * Make a minor formatting tweak in unmarshall_follower_list. * Rewrite the commit message. Thanks!

(0024602) neil (administrator) 2013-12-05 01:06	Closing; it's not entirely clear what originally caused wagnor's crash, but it was most likely the 1hp starcursed mass bug, fixed in 0.13-a0-2987-g83d4554 . The broken save should load now in 0.14 trunk, thanks to 0.14-a0-1307-gd1885e8 .

Issue History
Date Modified	Username	Field	Change
2013-06-03 15:59	wagnor	New Issue
2013-06-03 16:01	wagnor	File Added: crash-Wagnor-20130603-153920.txt
2013-06-03 16:03	wagnor	File Added: Wagnor.cs
2013-06-03 16:08	wagnor	Issue Monitored: wagnor
2013-06-03 16:11	neil	Note Added: 0023033
2013-06-03 16:12	neil	Note Edited: 0023033
2013-06-03 16:12	neil	Note Edited: 0023033
2013-06-03 19:44	wagnor	Note Added: 0023036
2013-06-03 21:16	neil	Note Added: 0023037
2013-06-04 01:50	neil	Summary	assert failed in 'tags.cc' => Saved with a dead monster in a transit list, now crashes on load
2013-08-09 21:35	neil	Note Added: 0023693
2013-08-09 22:28	neil	Note Edited: 0023693
2013-08-20 04:59	blackcustard	Note Added: 0023795
2013-08-20 05:02	blackcustard	File Added: 0001-DO-NOT-APPLY-TO-TRUNK.-Demonstrate-one-version-of-th.patch
2013-08-20 05:04	blackcustard	Note Edited: 0023795
2013-08-20 05:04	blackcustard	Note Edited: 0023795
2013-08-20 05:07	blackcustard	Note Edited: 0023795
2013-08-20 22:14	blackcustard	Note Edited: 0023795
2013-08-20 22:15	blackcustard	Note Edited: 0023795
2013-12-05 00:26	neil	Note Added: 0024600
2013-12-05 00:58	neil	Note Added: 0024601
2013-12-05 01:06	neil	Note Added: 0024602
2013-12-05 01:06	neil	Status	new => resolved
2013-12-05 01:06	neil	Fixed in Branch	=> 0.14 development branch
2013-12-05 01:06	neil	Resolution	open => done
2013-12-05 01:06	neil	Assigned To	=> neil

Relationships