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

Question

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?

Answer 1

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
    }
}

Answer 2

Actually with Boost Intrusive you worry about where an object is referenced all the time (because references are all over the place).

Maybe you just want weak_ptr. It also looks as though you're trying to implement garbage collection. Here's a slightly reworked idea using weak_ptr:

Live On Coliru

#include <vector>
#include <map>
#include <boost/weak_ptr.hpp>
#include <boost/make_shared.hpp>
#include <iostream>

using std::string;
using boost::shared_ptr;
using boost::weak_ptr;
using boost::make_shared;

struct Person {
    string name_;
    Person(string name) : name_(move(name)) {}

    using tag = weak_ptr<void>;

    tag track() const { return tracking_tag; }

  private:
    shared_ptr<void> tracking_tag = make_shared<char>('x');
};

template <typename T>
inline bool IsDeleted(weak_ptr<T> const& v) {
    return !v.lock();
}

enum favcolor { red, blue, pink, magenta, beige } favorite_color;

template <typename Map> size_t garbage_collect(Map& map)
{
    size_t collected = 0;
    for (auto it = map.begin(); it!=map.end();)
    {
        if (IsDeleted(it->first))
            ++collected, it = map.erase(it);
        else
            ++it;
    }

    return collected;
}

int main()
{
    std::vector<Person> people;

    for (auto&& name : { "John", "Mike", "Garbarek", "Milou", "Confucius", "Kiplat" })
        people.emplace_back(name);

    struct Properties {
        favcolor favorite_color;
        struct Car { string brand; int year; } vehicle;
    };

    std::map<Person::tag, Properties> associated {
        { people[0].track(), Properties { magenta, { "Chevy", 1986 } } },
        { people[2].track(), Properties { pink,    { "Kia",   2011 } } },
    };

    for (auto& p : people) std::cout << p.name_ << " "; std::cout << "\n";
    std::cout << "Defined properties: " << associated.size() << "\n";

    people.erase(std::remove_if(people.begin(), people.end(), [](Person const& p) { return p.name_[1] == 'o'; }), people.end());

    for (auto& p : people) std::cout << p.name_ << " "; std::cout << "\n";
    std::cout << "Defined properties before garbage collect: " << associated.size() << "\n";

    auto count = garbage_collect(associated);
    std::cout << "Defined properties after garbage collect: " << associated.size() << " (" << count << " collected)\n";
}

Output:

John Mike Garbarek Milou Confucius Kiplat 
Defined properties: 2
Mike Garbarek Milou Kiplat 
Defined properties before garbage collect: 2
Defined properties after garbage collect: 1 (1 collected)