beginh February 2016

shared pointer assertion fail after iteration in a loop

I am new to shared_ptr from boost and am considering to iterate over my set to get the best object. EDIT: added information about first_world

std::set<World::CPtr> first_world = ... // long call, but it gets a set of constant shared pointers to the class World, where various methods exist

   typedef boost::shared_ptr<World const> CPtr;
   World::CPtr best = *(first_world.begin());
   for (World::CPtr lo : first_world) {
     if (best->getValue() >= lo->getValue() ){
       best = lo;
     }
   }

Later I want to use that shared pointer, My program crashes with communicate that Assertion `px != 0' failed. I followed the rules from here, I mean I used a shared pointer as iterator in a loop but then I assign it to another pointer. Is that bad practice, is there better practice?

cout << "name is: " << best->getDefinition() << endl;

Answers


Mark Waterman February 2016

Nothing's blatantly wrong in what's pasted there, so there's probably going to be a mistake in the long call that creates the set.

For example, it would be easy to mess this up if raw pointers are involved when adding elements to the set. Consider this situation, a concrete illustration of a common mistake that's sort of alluded to in your Best Practices link:

std::set<World::CPtr> first_world;

World* pWorld = new World();

// Bad:
first_world.insert(World::CPtr(pWorld));
first_world.insert(World::CPtr(pWorld));

// Oops!! You now have two independently refcounted entries in first_world!

// emplace is just as deadly, but more subtle.
// Now you'll have three shared pointers in your set:
first_world.emplace(pWorld);

If you look through your entries in first_world and see duplicates then you'll know you're in trouble. To avoid mistakes like this, make sure you only construct shared_ptrs from other shared_ptrs (or boost::make_shared).

So that's tip #1: Avoid constructing shared_ptrs from a raw pointers. (That includes the this pointer if Worlds are adding themselves to your set... if you're doing that, better start googling enable_shared_from_this).

Now let's follow that guideline to get expected behavior:

std::set<World::CPtr> first_world;

World::CPtr spWorld1 = boost::make_shared<World>();
World::CPtr spWorld2{spWorld1};

first_world.insert(spWorld1);
first_world.insert(spWorld2);
// Just one element in first_world now, as expected.

Finally, a few (somewhat unrelated) suggestions:

  • std::set as you've declared it is only looking at the address of World objects on the heap when it compares entries. So if you have two different Worlds on the heap that are logically identical then they'll both have distinct entries in the set. Is that your intent?

Post Status

Asked in February 2016
Viewed 3,074 times
Voted 14
Answered 1 times

Search




Leave an answer