|Anonymous | Login||2018-12-15 10:54 UTC|
|My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0002048||VCMI||GUI - PreGame||public||2015-01-19 23:39||2016-11-11 08:22|
|Target Version||Fixed in Version||0.97c|
|Summary||0002048: reloading game causes crash if there was a path visible|
|Description||Reproducibility is actually "almost always", as sometimes it works; most probably caused by memory corruption.|
|Steps To Reproduce||* Load any map,|
* Click somewhere to define a short (1-3 tiles) path with your hero; (it seems that longer path rarely crash the game),
* Press menu -> load -> confirm -> choose another map,
* Game should crash during loading.
Reloading the same map generally doesn't crash the game -- possibly some pointers still reference "old" path from previous map?
0 CTerrainRect::showPath CAdvmapInterface.cpp 180 0x4544a1
1 CTerrainRect::show CAdvmapInterface.cpp 278 0x4547c8
2 CAdvMapInt::show CAdvmapInterface.cpp 751 0x45a116
3 CAdvMapInt::showAll CAdvmapInterface.cpp 673 0x459f28
4 CGuiHandler::totalRedraw CGuiHandler.cpp 157 0x42ab43
5 CGuiHandler::pushInt CGuiHandler.cpp 122 0x42cb82
6 CPlayerInterface::playerStartsTurn CPlayerInterface.cpp 2556 0x4d3f95
7 YourTurn::applyCl NetPacksClient.cpp 794 0x5113ea
8 CApplyOnCL<YourTurn>::applyOnClAfter Client.cpp 78 0x5b6363
9 CClient::handlePack Client.cpp 647 0x4b33cc
10 CClient::run Client.cpp 172 0x4b374b
11 boost::(anonymous namespace)::thread_start_function(void*)@4 0x5368d2
12 boost::(anonymous namespace)::ThreadProxy(void*)@4 0x5352e3
13 KERNEL32!BaseThreadInitThunk C:\Windows\syswow64\kernel32.dll 0x76e9338a
14 ntdll!RtlInitializeExceptionChain C:\Windows\SysWOW64\ntdll.dll 0x77d09f72
15 ntdll!RtlInitializeExceptionChain C:\Windows\SysWOW64\ntdll.dll 0x77d09f45
In the code, nextPos variable has broken values and indexes go waaay out of bounds.
|Tags||No tags attached.|
This one really annoying and likely have multiple duplicates, e.g: 0001771, 0001672 and likely many more include some my crash report.
I also guess this problem have same nature with other crash inside "CPathfinder::calculatePaths" on loading maps as those two likely use same features.
Fix in this PR seems to fix the problem for me. https://github.com/vcmi/vcmi/pull/87 [^]
Haven't checked other reports, but it's likely that these are the same issues.
|Should I add 'related to' for these other similar issues and/or add notes about possibly being fixed there?|
|Maybe, but after merge I can reproduce the issue just the same :/|
Strange. I was able to reproduce it nearly 100% of time and after that fix I no longer get this crash.
However, I've just found another problem related to reloading while testing now. When I reload between 2 saves (not any saves, just these two), it crashes 100% of time because of broken tile surface ptr. Not related to paths.
Does your crash point to showPath()?
I'm sorry, but are you sure that setting nullptr there is actually correct fix for this problem?
Of course it's fix the crash itself, but is that pointer have to be there after loading at all? Isn't this kind a hacky solution?
edited on: 2015-02-13 12:37
What I mean: there is multiple similar problems where game state incorrect after exiting to menu and back to game, e.g one with "CPathfinder::calculatePaths" where pointer's aren't the same. So I just worry there is some bigger flaw somewhere around and only fixing it's consequences may be not that good idea.
So I wonder may be there have to be feature that restore client state to the point like it's was just after startup to menu. Or something like that.
This crash is/was caused by the same problem as 0001953. When you reload the game (load a save after playing a map), player interface reuses CAdvMapInt instance instead of creating new one.
Current path isn't set until it's created by user or deserialized from save state (later I think). That's why after reload, terrain.currentPath points to "path" from previous game.
|The other solution would be to recreate adv map interface every time you start the game. I don't know if there's a valid reason to keep and reuse an instance (probably high cost of creating new one?).|
|It's easier to manage if you create new interface every time. This can't be costly as you do that on every new game anyway.|
|I think it's better solution to create as much as technically possible from scratch to avoid such conflicts. Crashes like this one are way too annoying and sometimes hard to debug so better to avoid them completely.|
I can try changing it (probably not today though).
But still, I'm not sure if it's safe to recreate the instance; sounds risky. ;)
|If we decide to continue reusing advMapInt objects, then we should once more review CAdvMapInt::restoreState().|
|And I also don`t like the existence of terrain.currentPath (as variable) at all. This will surely cause more crashes in future.|
|2015-01-19 23:39||Fay||New Issue|
|2015-01-19 23:39||Fay||Status||new => assigned|
|2015-01-19 23:39||Fay||Assigned To||=> Tow|
|2015-02-10 22:43||SXX||Note Added: 0005445|
|2015-02-13 10:24||Fay||Note Added: 0005453|
|2015-02-13 10:24||Fay||Assigned To||Tow => Fay|
|2015-02-13 11:03||Fay||Status||assigned => resolved|
|2015-02-13 11:03||Fay||Resolution||open => fixed|
|2015-02-13 11:03||Fay||Fixed in Version||=> 0.97c|
|2015-02-13 11:05||Fay||Note Added: 0005455|
|2015-02-13 11:15||Warmonger||Note Added: 0005456|
|2015-02-13 11:40||Fay||Note Added: 0005457|
|2015-02-13 12:32||SXX||Note Added: 0005458|
|2015-02-13 12:36||SXX||Note Added: 0005459|
|2015-02-13 12:37||SXX||Note Edited: 0005459||View Revisions|
|2015-02-13 13:08||Fay||Note Added: 0005460|
|2015-02-13 13:10||Fay||Note Added: 0005461|
|2015-02-13 13:35||Warmonger||Note Added: 0005462|
|2015-02-13 14:43||SXX||Note Added: 0005463|
|2015-02-13 15:42||Fay||Note Added: 0005465|
|2015-02-13 16:26||AVS||Note Added: 0005467|
|2015-02-13 16:39||AVS||Note Added: 0005468|
|2015-11-18 03:15||SXX||Relationship added||related to 0001954|
|2016-11-11 08:22||SXX||Status||resolved => closed|
|Copyright © 2000 - 2018 MantisBT Team|