Just wanted to point out that there is a bug report that seems possibly related, or at least describes a different path to segfault:
https://gna.org/bugs/?20071 On Fri, Apr 4, 2014 at 8:10 PM, Alexander van Gessel <ai0...@gmail.com>wrote: > On Fri, Apr 4, 2014 at 7:28 PM, chris beck <beck...@gmail.com> wrote: >> >> With regards to the reference counting / possible memory corruption: The >> unit_map does indeed have some reference counting built in, but that is >> only for unit_map::iterator objects afaict. Many of the wesnoth modules >> currently interact with the unit_map by using "extract", which extracts a >> naked pointer to the unit and "transfers ownership" to the recipient. From >> what I understood, the whiteboard in some circumstances takes pointers like >> this and wraps them in boost shared pointers for its own use. But I guess >> that this means we have two disjoint reference counting systems, so there >> could potentially still be pointer problems between modules. E.g. what >> happens if a unit is owned by the unit_map and being animated, but during >> the animation it is transferred to the whiteboard / some other module... ? >> Since animations don't use the reference counted pointer but rather a raw >> pointer I think it may result in segfault if the whiteboard then destroys >> the unit. (It's also worth noting that the unit_map has been failing its >> unit tests regarding invalidating iterators that are supposed to be dead, >> so this might also somehow be a contributing factor in some scenarios.) >> > I took a look at this, and there are only 3 systems where > unit_map::extract is called. > -Lua bindings. It is called by wesnoth.extract_unit, which removes a unit > from its current owner and stores it in a garbage-collected lua object. > -In unit.cpp, various temporary_unit_foo classes extract a unit, store it > in a raw pointer, and put it back in their destructor. > -Three whiteboard actions, which like the temporary_unit_foo classes, make > modifications to various gamestate objects. Unlike the temporary_unit_foo > classes though, they store it in a std::auto_ptr<unit>. This pointer seems > to be non-NULL whenever the unit_map does not own the unit. The action is > stored, wrapped in a shared_ptr, in some multi_index boost structure, so > I'm quite unsure about object lifetimes here. > > Note that the actual pointer can also be (and in certain places, is) > extracted by doing &*itor. > > A possible solution that I mulled over was to have different modules pass >> unit_pods around (the class used internally by unit_map as a >> reference-counted pointer struct) or trying to make some of them use the >> unit_map iterator type rather than raw pointers to units that they do not >> own. But finally I concluded that since I'm not a real C++ programmer, >> anything I do here would most likely be flawed and need to be redone -- >> really IMO we should get a bright GSoC student who knows a lot about STL to >> figure out a modern smart pointer system we can use and implement it. >> > As long as you take ownership of the object, you can wrap it however you > like. The unit_map's unit_pod has the purpose of preventing iterator > invalidation. The pod refcounts the iterators and is only removed from the > map when there are no other references to it remaining. > > Regarding unit animations. They indeed take a raw pointer that is owned by > someone else and assume the unit continues to exist during the length of > the animation. Furthermore, animations use state *owned by the unit*, so > they do not support reentrancy. (see bug #18921) > >> > _______________________________________________ > Wesnoth-dev mailing list > Wesnoth-dev@gna.org > https://mail.gna.org/listinfo/wesnoth-dev > >
_______________________________________________ Wesnoth-dev mailing list Wesnoth-dev@gna.org https://mail.gna.org/listinfo/wesnoth-dev
