c++c++11booststdintrusive-containers

Trying to learn boost::intrusive Q3 - When storing pointers in ICs, should I use smart_pointer?


I have progressed greatly in my understanding of intrusive containers. I have a program that runs for a "while", and then on a line of code like this delete *it; (see below):

....

                // :  public list_base_hook< void_pointer< ip::offset_ptr<void> > >
class OneDepthPrice : public list_base_hook<link_mode<auto_unlink>> // This is a derivation hook
{
public:
    Provider provider;
    Price price;
public:
    list_member_hook<link_mode<auto_unlink>> member_hook_; // This is a member hook

    OneDepthPrice(Provider prov, Price p) : provider(prov), price(p) {}
};

...

std::vector<OneDepthPrice *>& vecPrices

for (auto it = vecPrices.begin(); it != vecPrices.end();  ++it)
{
    auto& e = *it;
#if DEBUG
    std::cout << e->provider.name << "\n" << std::flush;
#endif
    if(e->provider.name == newPrices.provider.name)
    {
        delete *it; //This is the offending line in the stack trace in the debugger.

        it = vecPrices.erase(it);
    }
}

the program crashes with this stack trace:

#0 0x407ddd boost::intrusive::list_node_traits<void*>::set_next(n=@0x7fffffffe2b8: 0x0, 
next=@0x7fffffffe2b0: 0x706860) (/usr/local/include/boost/intrusive/detail/list_node.hpp:64)
#1 0x409189 boost::intrusive::circular_list_algorithms<boost::intrusive::list_node_traits<void*> >::unlink(this_node=@0x7fffffffe2e8: 0x706830) (/usr/local/include/boost/intrusive/circular_list_algorithms.hpp:140)
#2 0x407e2a boost::intrusive::generic_hook<boost::intrusive::get_list_node_algo<void*>, boost::intrusive::default_tag, (boost::intrusive::link_mode_type)2, (boost::intrusive::base_hook_type)1>::unlink(this=0x706830) (/usr/local/include/boost/intrusive/detail/generic_hook.hpp:180)
#3 0x406b5c boost::intrusive::detail::destructor_impl<boost::intrusive::generic_hook<boost::intrusive::get_list_node_algo<void*>, boost::intrusive::default_tag, (boost::intrusive::link_mode_type)2, (boost::intrusive::base_hook_type)1> >(hook=...) (/usr/local/include/boost/intrusive/detail/utilities.hpp:371)
#4 0x405b13 boost::intrusive::generic_hook<boost::intrusive::get_list_node_algo<void*>, boost::intrusive::default_tag, (boost::intrusive::link_mode_type)2, (boost::intrusive::base_hook_type)1>::~generic_hook(this=0x706830, __in_chrg=<optimized out>) (/usr/local/include/boost/intrusive/detail/generic_hook.hpp:160)
#5 0x40534a boost::intrusive::list_base_hook<boost::intrusive::link_mode<(boost::intrusive::link_mode_type)2>, void, void>::~list_base_hook(this=0x706830, __in_chrg=<optimized out>) (/usr/local/include/boost/intrusive/list_hook.hpp:86)
#6 0x405546 OneDepthPrice::~OneDepthPrice(this=0x706830, __in_chrg=<optimized out>) (/home/idf/Documents/TestCPPArrays/TestCPPArrays.cpp:86)
#7 0x403728 UpdateBunchTogether(vectogether=..., vecPrices=..., newPrices=...) (/home/idf/Documents/TestCPPArrays/TestCPPArrays.cpp:291)
#8 0x404765 main() (/home/idf/Documents/TestCPPArrays/TestCPPArrays.cpp:558)

This is a strange bug because the program is not multi-threaded but it runs for a while without a hitch. I am not sure what is happening, but maybe I need to use smart_pointers?


Solution

  • Moving my comment to an answer. When you do:

    for (auto it = vecPrices.begin(); it != vecPrices.end();  ++it)
    {
        auto& e = *it;
        if(e->provider.name == newPrices.provider.name)
        {
            delete *it; //This is the offending line in the stack trace in the debugger.
    
            it = vecPrices.erase(it);
        }
    }
    

    You correctly update the iterator when you erase, but then you unconditionally increment it. This is bad for two reasons: you could fail to delete the next object if you need to, and if erase() returns end() then you just walked past the end of your vector.

    To erase safely, you need to do:

    for (auto it = vecPrices.begin(); it != vecPrices.end();  /* nothing */)
    {
        auto& e = *it;
        if(e->provider.name == newPrices.provider.name)
        {
            delete *it; //This is the offending line in the stack trace in the debugger.
    
            it = vecPrices.erase(it);
        }
        else 
        {
            ++it; // this is where we increment
        }
    }