MantisBT
My View | View Issues | Change Log | Roadmap
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0000808VCMIGUI - Battlespublic2011-09-10 16:142014-05-30 17:40
Reportervaderas 
Assigned ToIvan 
PrioritynormalSeveritycrashReproducibilityalways
StatusclosedResolutionfixed 
Platformwindows 7OSOS Version
Product Version0.86 
Target VersionFixed in Version0.87 
Summary0000808: unable to upgrade units
Descriptionalready created units could not be upgraded (tried it with necropolis ->vampires)
TagsNo tags attached.
Attached Filestxt file icon VCMI_Client_log.txt [^] (232,603 bytes) 2011-09-10 16:36 [Show Content]
patch file icon UpgradeCreature.patch [^] (360 bytes) 2011-09-25 16:41 [Show Content]

- Relationships

-  Notes
(0002004)
Warmonger (administrator)
2011-09-10 16:26
edited on: 2011-09-10 16:28

 Details? What do you mean by "already created units"?

Is config\creatures.json file present?
(0002005)
vaderas (reporter)
2011-09-10 16:35

 config\creatures.json is present,
i mean when i create units that are not upgraded and try to upgrade them from their own window, it crashes. The log doesn't show anything useful, everything seems normal. I haven't tried to debug it. Do you want me to try solve it and post the solution?
(0002006)
Warmonger (administrator)
2011-09-10 16:47

 Well, I tried to buy Vampires on Arrogance 3 and upgraded them to Vampire Lords successfully. So it's not simply reproductible.
(0002007)
vaderas (reporter)
2011-09-10 16:50

 ok. ill try to debug it and see the reason im having it and ill report back
(0002008)
vaderas (reporter)
2011-09-10 17:29

 apparently this happens when trying to delete the info window, that is it happens even if i cancel the upgrade (by pressing cancel). the line that throws exception is in GUIClasses.cpp Line 2225
CCreInfoWindow::~CCreInfoWindow()
{
     for(int i=0; i<upgResCost.size();i++)
         delete upgResCost[i];
}
 Access violation reading location 0x25530580

Any thoughts?

ps1: handle of upgResCost[0] = 0x12a982c0 = (access_violation_address)/2 (!!), so the unhandled exception happens due to reading uncharted territory
ps2: im using vc++ 2010 compiler
(0002010)
ubuntux (developer)
2011-09-13 02:21

 To reproduce, you must have the regular creature info windows, not the enhanced one (classicCreatureWindow=1;)
Recruit a non-upgraded creature, double click on it to open the info, select upgrade but click on cancel -> memory corruption.

Here's a valgrind trace for that one:

==7988== Thread 4:
==7988== Invalid read of size 8
==7988== at 0x81363A: CCreInfoWindow::~CCreInfoWindow() (GUIClasses.cpp:2228)
==7988== by 0x7F2592: CGuiHandler::popIntTotally(IShowActivable*) (GUIBase.cpp:71)
==7988== by 0x8137A0: CCreInfoWindow::close() (GUIClasses.cpp:2240)
==7988== by 0x87DB20: boost::_mfi::mf0<void, CCreInfoWindow>::operator()(CCreInfoWindow*) const (mem_fn_template.hpp:49)
==7988== by 0x87991F: void boost::_bi::list1<boost::_bi::value<CCreInfoWindow*> >::operator()<boost::_mfi::mf0<void, CCreInfoWindow>, boost::_bi::list0>(boost::_bi::type<void>, boost::_mfi::mf0<void, CCreInfoWindow>&, boost::_bi::list0&, int) (bind.hpp:253)
==7988== by 0x876070: boost::_bi::bind_t<void, boost::_mfi::mf0<void, CCreInfoWindow>, boost::_bi::list1<boost::_bi::value<CCreInfoWindow*> > >::operator()() (bind_template.hpp:20)
==7988== by 0x8721FA: boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<void, boost::_mfi::mf0<void, CCreInfoWindow>, boost::_bi::list1<boost::_bi::value<CCreInfoWindow*> > >, void>::invoke(boost::detail::function::function_buffer&) (function_template.hpp:153)
==7988== by 0x52375E: boost::function0<void>::operator()() const (function_template.hpp:1013)
==7988== by 0x52C59A: CFunctionList<void ()()>::operator()() const (FunctionList.h:64)
==7988== by 0x52627A: AdventureMapButton::clickLeft(boost::logic::tribool, bool) (AdventureMapButton.cpp:168)
==7988== by 0x7F3660: CGuiHandler::handleEvent(SDL_Event*) (GUIBase.cpp:262)
==7988== by 0x7F2A51: CGuiHandler::handleEvents() (GUIBase.cpp:154)
==7988== Address 0x20cd0ae0 is 0 bytes inside a block of size 152 free'd
==7988== at 0x4C27A83: operator delete(void*) (vg_replace_malloc.c:387)
==7988== by 0x8010FD: SComponent::~SComponent() (GUIClasses.cpp:832)
==7988== by 0x7F5AD8: CIntObject::~CIntObject() (GUIBase.cpp:633)
==7988== by 0x8024A7: CSimpleWindow::~CSimpleWindow() (GUIClasses.cpp:973)
==7988== by 0x7FE2B9: CInfoWindow::~CInfoWindow() (GUIClasses.cpp:553)
==7988== by 0x7F2592: CGuiHandler::popIntTotally(IShowActivable*) (GUIBase.cpp:71)
==7988== by 0x7FDE52: CInfoWindow::close() (GUIClasses.cpp:536)
==7988== by 0x881B8A: boost::_mfi::mf0<void, CInfoWindow>::operator()(CInfoWindow*) const (mem_fn_template.hpp:49)
==7988== by 0x87CAD3: void boost::_bi::list1<boost::_bi::value<CInfoWindow*> >::operator()<boost::_mfi::mf0<void, CInfoWindow>, boost::_bi::list0>(boost::_bi::type<void>, boost::_mfi::mf0<void, CInfoWindow>&, boost::_bi::list0&, int) (bind.hpp:253)
==7988== by 0x878BAA: boost::_bi::bind_t<void, boost::_mfi::mf0<void, CInfoWindow>, boost::_bi::list1<boost::_bi::value<CInfoWindow*> > >::operator()() (bind_template.hpp:20)
==7988== by 0x875651: boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<void, boost::_mfi::mf0<void, CInfoWindow>, boost::_bi::list1<boost::_bi::value<CInfoWindow*> > >, void>::invoke(boost::detail::function::function_buffer&) (function_template.hpp:153)
==7988== by 0x52375E: boost::function0<void>::operator()() const (function_template.hpp:1013)


So it's a double free. I suppose the second free should go away but I don't know whether that's the right fix.
(0002058)
Gesh (reporter)
2011-09-25 16:41

 So I've investigated the problem for awhile and I think its solved now. So what happens.
When we click the upgrade button an yes/no dialog is shown. We have this call

cfl = boost::bind(&CPlayerInterface::showYesNoDialog, LOCPLINT, CGI->generaltexth->allTexts[207], boost::ref(upgResCost), fs, 0, true);

which effectively assembles the yes/no dialog. The components of the dialog are upgResCost. Note the last parameter - true. This parameter tells the GUI system that a CIntObject should delete its components on destruction - and the yes/no popup is CInfoWindow, which is ultimatly a CIntObject and when its closed upgResCost gets deleted by the CIntObject's destructor. Then CCreInfoWindow's destructor kicks in, which in turn deletes upgResCost a second time, hence the crash.

The solution is twofold:
a) change the last param in the above call to false
b) clean the destructor of CCreInfoWindow

I took the second approach since its more consistent with the other occurences of the yes/no dialog's construction, but I guess it doesnt really matter. Patch is uploaded.

Big Cheers to Ubuntux for the double delete hint. :)
(0002063)
Zamolxis (viewer)
2011-09-25 21:10

 Many thanks for the contribution Gesh!

I'm assigning this to Ubuntux. I see he has been busy with it already, so he might want to double check the fix and upload the change (but if s/o else would like to do it sooner, it's fine as well of course).
(0002067)
ubuntux (developer)
2011-09-26 00:07
edited on: 2011-09-26 00:08

 I'm not convinced that is the right fix. Whether true or false is given to boost::bind, the object should not be freed twice.
(0002076)
Ivan (developer)
2011-09-29 14:36

 I'll check this
(0002082)
Ivan (developer)
2011-10-08 17:01

 Fixed in rev 2347.

- Issue History
Date Modified Username Field Change
2011-09-10 16:14 vaderas New Issue
2011-09-10 16:14 vaderas Status new => assigned
2011-09-10 16:14 vaderas Assigned To => Tow dragon
2011-09-10 16:26 Warmonger Note Added: 0002004
2011-09-10 16:26 Warmonger Assigned To Tow dragon =>
2011-09-10 16:28 Warmonger Note Edited: 0002004 View Revisions
2011-09-10 16:29 Warmonger Status assigned => new
2011-09-10 16:35 vaderas Note Added: 0002005
2011-09-10 16:36 vaderas File Added: VCMI_Client_log.txt
2011-09-10 16:47 Warmonger Note Added: 0002006
2011-09-10 16:50 vaderas Note Added: 0002007
2011-09-10 17:29 vaderas Note Added: 0002008
2011-09-13 02:21 ubuntux Note Added: 0002010
2011-09-25 16:41 Gesh Note Added: 0002058
2011-09-25 16:41 Gesh File Added: UpgradeCreature.patch
2011-09-25 21:04 Zamolxis Status new => assigned
2011-09-25 21:04 Zamolxis Assigned To => ubuntux
2011-09-25 21:10 Zamolxis Note Added: 0002063
2011-09-26 00:07 ubuntux Note Added: 0002067
2011-09-26 00:08 ubuntux Note Edited: 0002067 View Revisions
2011-09-26 00:08 ubuntux Assigned To ubuntux => Tow dragon
2011-09-29 14:36 Ivan Assigned To Tow dragon => Ivan
2011-09-29 14:36 Ivan Note Added: 0002076
2011-10-08 17:01 Ivan Note Added: 0002082
2011-10-08 17:01 Ivan Status assigned => resolved
2011-10-08 17:01 Ivan Fixed in Version => 0.89
2011-10-08 17:01 Ivan Resolution open => fixed
2014-05-30 17:40 beegee Status resolved => closed

Site | Forums | Wiki | Slack | GitHub

Copyright © 2000 - 2024 MantisBT Team
Hosting provided by DigitalOcean