MantisBT - VCMI
View Issue Details
0001444VCMIAI - Adventure Mappublic2013-09-06 11:562014-06-22 15:07
Povelitel 
Tow 
highcrashalways
closedfixed 
windowsxpsp3
0.93b 
0.94 
0001444: crash turn AI
End turn.
Client crash. Server crash.
No tags attached.
has duplicate 0001432closed Tow crash turn AI 
has duplicate 0001439closed Tow crash turn AI 
has duplicate 0001452closed Tow crash turn AI 
has duplicate 0001471closed Tow crash turn AI 
has duplicate 0001467closed Tow crash turn AI 
has duplicate 0001474closed Tow crash turn AI 
has duplicate 0001475closed Tow crash turn AI 
related to 0001449closed Tow crash turn AI 
related to 0001472closed Tow crash turn AI 
rar games.rar (799,416) 2013-09-06 11:56
https://bugs.vcmi.eu/file_download.php?file_id=1463&type=bug
Issue History
2013-09-06 11:56PovelitelNew Issue
2013-09-06 11:56PovelitelStatusnew => assigned
2013-09-06 11:56PovelitelAssigned To => Tow
2013-09-06 11:56PovelitelFile Added: games.rar
2013-09-06 20:39TowNote Added: 0003937
2013-09-06 21:55IvanNote Added: 0003938
2013-09-08 16:31TowRelationship addedhas duplicate 0001432
2013-09-13 22:41TowRelationship addedhas duplicate 0001439
2013-09-14 19:21TowRelationship addedrelated to 0001449
2013-09-14 22:52TowRelationship addedhas duplicate 0001452
2013-09-27 19:48TowNote Added: 0004039
2013-09-28 10:17TowRelationship addedrelated to 0001472
2013-09-28 10:18TowRelationship addedhas duplicate 0001471
2013-09-28 10:18TowRelationship addedhas duplicate 0001467
2013-09-28 10:24TowRelationship addedhas duplicate 0001474
2013-09-28 10:29TowRelationship addedhas duplicate 0001475
2013-10-09 11:00PovelitelNote Added: 0004069
2013-10-10 17:35TowNote Added: 0004072
2013-10-10 17:35TowStatusassigned => resolved
2013-10-10 17:35TowFixed in Version => 0.94
2013-10-10 17:35TowResolutionopen => fixed
2014-06-22 13:38PovelitelNote Added: 0004776
2014-06-22 15:07KantorNote Added: 0004796
2014-06-22 15:07KantorStatusresolved => closed

Notes
(0003937)
Tow   
2013-09-06 20:39   
Interesting…


Crash happens in CGameHandler::buildStructure on iterating over the buildings list:
    for(auto & build : t->town->buildings)
    {
        if(t->hasBuilt(build.first))
            buildingsThatWillBe.push_back(build.second);
        else if(build.second->mode == CBuilding::BUILD_AUTO) //not built auto building
            remainingAutoBuildings.push_back(build.second);
    }

The buildings map contains following entries:

+ [0] ({num=MAGES_GUILD_1 (0) }, {ptr=0x03aeda30 {name="Mage Guild Level 1" description="Entering the Mage Guild will allow a visiting hero to learn the spells kept within." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [1] ({num=MAGES_GUILD_2 (1) }, {ptr=0x03aee570 {name="Mage Guild Level 2" description="Entering the Mage Guild will allow a visiting hero to learn the spells kept within." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [2] ({num=MAGES_GUILD_3 (2) }, {ptr=0x03aef410 {name="Mage Guild Level 3" description="Entering the Mage Guild will allow a visiting hero to learn the spells kept within." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [3] ({num=MAGES_GUILD_4 (3) }, {ptr=0x03aeea80 {name="Mage Guild Level 4" description="Entering the Mage Guild will allow a visiting hero to learn the spells kept within." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [4] ({num=TAVERN (5) }, {ptr=0x03aee450 {name="Tavern" description="The Tavern increases morale for troops defending the city." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [5] ({num=SHIPYARD (6) }, {ptr=0x03aee4e0 {name="Shipyard" description="The Shipyard allows you to purchase ships." town=0x043ee5d0 {...} ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [6] ({num=FORT (7) }, {ptr=0x03aeeba0 {name="Fort" description="The Fort provides your town with defensive walls." town=0x043ee5d0 {...} ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [7] ({num=CITADEL (8) }, {ptr=0x03aef2f0 {name="Citadel" description="Including a 50% increase to base creature growth, the Citadel adds a keep, and other terrain obstacles, to a town's defenses." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [8] ({num=CASTLE (9) }, {ptr=0x03aee960 {name="Castle" description="The Castle adds two arrow towers, fortifies your town's defenses, and doubles base creature growth." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [9] ({num=VILLAGE_HALL (10) }, {ptr=0x03aeef00 {name="Village Hall" description="The Village Hall allows you to purchase town structures and earns your kingdom 500 gold per day." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [10] ({num=TOWN_HALL (11) }, {ptr=0x03aeed50 {name="Town Hall" description="The Town Hall allows you to purchase town structures and earns your kingdom 1000 gold per day." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [11] ({num=CITY_HALL (12) }, {ptr=0x03aef380 {name="City Hall" description="The City Hall allows you to purchase town structures and earns your kingdom 2000 gold per day." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [12] ({num=CAPITOL (13) }, {ptr=0x03aeede0 {name="Capitol" description="The Capitol earns your kingdom 4000 gold per day." town=...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [13] ({num=MARKETPLACE (14) }, {ptr=0x03aeee70 {name="Marketplace" description="With the Marketplace, you can buy and sell resources (exchange rates increase with each Marketplace you own)." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [14] ({num=RESOURCE_SILO (15) }, {ptr=0x03aef4a0 {name="Resource Silo" description="The Resource Silo provides you with an additional +1 wood and +1 ore each day." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [15] ({num=BLACKSMITH (16) }, {ptr=0x03aee3c0 {name="Blacksmith" description="The Blacksmith provides your armies with Ballistas." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [16] ({num=SPECIAL_1 (17) }, {ptr=0x03aee690 {name="Lighthouse" description="The lighthouse grants additional mobility to any ships you control." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [17] ({num=HORDE_1 (18) }, {ptr=0x03aef1d0 {name="Griffin Bastion" description="The Griffin Bastion increases Griffin production by 3 per week." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [18] ({num=HORDE_1_UPGR (19) }, {ptr=0x03aee9f0 {name="Griffin Bastion" description="The Griffin Bastion increases Royal Griffin production by 3 per week." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [19] ({num=SHIP (20) }, {ptr=0x03aeeb10 {name="Shipyard" description="The Shipyard allows you to purchase ships." town=0x043ee5d0 {...} ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [20] ({num=SPECIAL_2 (21) }, {ptr=0x03aee720 {name="Stables" description="The Stables increase the land movement rate of any visiting hero." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [21] ({num=SPECIAL_3 (22) }, {ptr=0x03aee7b0 {name="Brotherhood of the Sword" description="During a siege, the Brotherhood of the Sword increases troop morale by +2." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [22] ({num=GRAIL (26) }, {ptr=0x03aee840 {name="Colossus" description="The presence of the Colossus increases weekly creature generation by 50%, provides your kingdom with an additional 5000 gold each day, and increases the morale of all allied heroes by +2." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [23] ({num=EXTRA_TOWN_HALL (27) }, {ptr=0x00000000 {name= description= town=??? ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [24] ({num=EXTRA_CITY_HALL (28) }, {ptr=0x00000000 {name= description= town=??? ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [25] ({num=DWELL_FIRST (30) }, {ptr=0x03aeef90 {name="Guardhouse" description="The Guardhouse allows you to recruit Pikemen." town=...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [26] ({num=DWELL_LVL_2 (31) }, {ptr=0x03aef020 {name="Archers' Tower" description="The Archers' Tower allows you to recruit Archers." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [27] ({num=DWELL_LVL_3 (32) }, {ptr=0x03aef0b0 {name="Griffin Tower" description="The Griffin Tower allows you to recruit Griffins." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [28] ({num=DWELL_LVL_4 (33) }, {ptr=0x03aef140 {name="Barracks" description="The Barracks allow you to recruit Swordsmen." town=0x043ee5d0 {...} ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [29] ({num=DWELL_LVL_5 (34) }, {ptr=0x03aef650 {name="Monastery" description="The Monastery allows you to recruit Monks." town=0x043ee5d0 {...} ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [30] ({num=DWELL_LVL_6 (35) }, {ptr=0x03aefec0 {name="Training Grounds" description="The Training Grounds allow you to recruit Cavaliers." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [31] ({num=DWELL_LAST (36) }, {ptr=0x03aeff50 {name="Portal of Glory" description="The Portal of Glory allows you to recruit Angels." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [32] ({num=DWELL_UP_FIRST (37) }, {ptr=0x03aef5c0 {name="Upg. Guardhouse" description="The Guardhouse allows you to recruit Halberdiers." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [33] ({num=DWELL_LVL_2_UP (38) }, {ptr=0x03aeffe0 {name="Upg. Archers' Tower" description="The Archers' Tower allows you to recruit Marksmen." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [34] ({num=DWELL_LVL_3_UP (39) }, {ptr=0x03aef530 {name="Upg. Griffin Tower" description="The Griffin Tower allows you to recruit Royal Griffins." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [35] ({num=DWELL_LVL_4_UP (40) }, {ptr=0x03aefe30 {name="Upg. Barracks" description="The Barracks allow you to recruit Crusaders." town=...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [36] ({num=DWELL_LVL_5_UP (41) }, {ptr=0x03aef800 {name="Upg. Monastery" description="The Monastery allows you to recruit Zealots." town=...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [37] ({num=DWELL_LVL_6_UP (42) }, {ptr=0x03aef920 {name="Upg. Training Grounds" description="The Training Grounds allow you to recruit Champions." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >
+ [38] ({num=DWELL_UP_LAST (43) }, {ptr=0x03aefda0 {name="Upg. Portal of Glory" description="The Portal of Glory allows you to recruit Arch Angels." ...} }) std::pair<BuildingID const ,ConstTransitivePtr<CBuilding> >

The EXTRA_TOWN_HALL entries have nullptr stored and iterating over them is causing the crash.
The question is how did they get in the first place. Something must have accesed the map with using their IDs but I'm not sure what.
Interestingly, these entries only appear in the server copy of the state, the client's one is fine.
(0003938)
Ivan   
2013-09-06 21:55   
I noticed that CGTownInstance::CTown is not marked as "const" as well as several places where CTown::buildings may be accessed from non-const CTown

Don't know if this will fix this bug or not but I'll commit my changes here anyway - this is certainly dangerous.
(0004039)
Tow   
2013-09-27 19:48   
Revision r3554 brings a number of changes related to this issue:
* corrupted savegmae will be fixed on loading
* assert will cause crash when the bogus entries are added to the builtBuildings list + assert that all the list are good at the game start

If the issue is still here (might have been fixed by Ivan changes), this should help us catch the exact problem.
(0004069)
Povelitel   
2013-10-09 11:00   
I can not understand this bug can not be cured? or what it is, why you do not fixed it? Or is this incredibly difficult?
(0004072)
Tow   
2013-10-10 17:35   
Actually it was fixed in r3567, I forgot to update this item. Sorry for that.
(0004776)
Povelitel   
2014-06-22 13:38   
Kantor, look, can can close them?
(0004796)
Kantor   
2014-06-22 15:07   
This one may be closed.