MantisBT

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0002048VCMIGUI - PreGamepublic2015-01-19 23:392016-11-11 08:22
ReporterFay 
Assigned ToFay 
PrioritynormalSeveritycrashReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS VersionWin7x64
Product Version0.97b 
Target VersionFixed in Version0.97c 
Summary0002048: reloading game causes crash if there was a path visible
DescriptionReproducibility 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?
Additional InformationStack:
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
16 ??

In the code, nextPos variable has broken values and indexes go waaay out of bounds.
TagsNo tags attached.
Attached Files

- Relationships
related to 0001954resolvedSXX vcmiclient crash after same save loaded 2nd time 

-  Notes
(0005445)
SXX (administrator)
2015-02-10 22:43

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.
(0005453)
Fay (developer)
2015-02-13 10:24

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.
(0005455)
Fay (developer)
2015-02-13 11:05

Should I add 'related to' for these other similar issues and/or add notes about possibly being fixed there?
(0005456)
Warmonger (administrator)
2015-02-13 11:15

Maybe, but after merge I can reproduce the issue just the same :/
(0005457)
Fay (developer)
2015-02-13 11:40

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()?
(0005458)
SXX (administrator)
2015-02-13 12:32

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?
(0005459)
SXX (administrator)
2015-02-13 12:36
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.

(0005460)
Fay (developer)
2015-02-13 13:08

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.
(0005461)
Fay (developer)
2015-02-13 13:10

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?).
(0005462)
Warmonger (administrator)
2015-02-13 13:35

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.
(0005463)
SXX (administrator)
2015-02-13 14:43

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.
(0005465)
Fay (developer)
2015-02-13 15:42

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. ;)
(0005467)
AVS (administrator)
2015-02-13 16:26

If we decide to continue reusing advMapInt objects, then we should once more review CAdvMapInt::restoreState().
(0005468)
AVS (administrator)
2015-02-13 16:39

And I also don`t like the existence of terrain.currentPath (as variable) at all. This will surely cause more crashes in future.

- Issue History
Date Modified Username Field Change
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

Site | Forums | Wiki | Slack | GitHub


Copyright © 2000 - 2018 MantisBT Team
Hosting provided by DigitalOcean