|
Notes |
|
|
(0002004)
|
|
Warmonger
|
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?
|
|
|
|
|
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? |
|
|
|
|
|
Well, I tried to buy Vampires on Arrogance 3 and upgraded them to Vampire Lords successfully. So it's not simply reproductible. |
|
|
|
|
|
ok. ill try to debug it and see the reason im having it and ill report back |
|
|
|
|
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 |
|
|
|
|
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
|
|
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. :) |
|
|
|
|
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
|
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
|
|
2011-09-29 14:36
|
|
|
|
|
(0002082)
|
|
Ivan
|
|
2011-10-08 17:01
|
|
|
VCMI_Client_log.txt (232,603) 2011-09-10 16:36