BOOST_FOREACH Iteration over boost::shared_ptr<list>

Question

I'm doing something similar to this item Correct BOOST_FOREACH usage?

However, my returned list is wrapped in a boost::shared_ptr. If I do not assign the list to a variable before the BOOST_FOREACH loop, I get a crash at runtime as the list is getting destructed as it is a temporary.

boost::shared_ptr< list<int> > GetList()
{
    boost::shared_ptr< list<int> > myList( new list<int>() );
    myList->push_back( 3 );
    myList->push_back( 4 );
    return myList;
}

Then later..

// Works if I comment out the next line and iterate over myList instead
// boost::shared_ptr< list<int> > myList = GetList();

BOOST_FOREACH( int i, *GetList() ) // Otherwise crashes here
{
    cout << i << endl;
}

I would like to be able to use the above without having to introduce a variable 'myList'. Is this possible?

Manolo

How does it crash? With any assertion fail? Or something else? – Gaudreau

Unhandled exception at 0x00426692 in BOOSTFOREACH.exe: 0xC0000005: Access violation reading location 0xfeeefeee. – Manolo

I'm assuming the crash is because the list<int> has been destroyed (I traced through the destructor) but BOOST_FOREACH craps out when trying to call list<int>::begin() – Manolo

You're dereferencing the shared_ptr but never copying it to a new shared_ptr so the reference count will reach 0 and it will be destroyed. If you were to try doing auto list = GetList(); on the line before and then BOOST_FOREACH( int i, *list ) it should fix that specific error. – Agiotage

Yeah, it looks like it's something I'll need to do in the end. I started using BOOST_FOREACH to avoid having to do all this in the first place. – Manolo

Returning the shared_ptr by value causes the copy constructor to run, which will increment the internal object count. This should take care of preserving the list. – Pooh

The shared_ptr is being returned by value – Manolo

Answer 1

Ok, the 'Best Practice' for shared_ptr mentions to avoid using unnamed temporaries:

http://www.boost.org/doc/libs/release/libs/smart_ptr/shared_ptr.htm#BestPractices

Avoid using unnamed shared_ptr temporaries to save typing; to see why this is dangerous, consider this example:

void f(shared_ptr<int>, int); int g();

void ok() {
    shared_ptr<int> p(new int(2));
    f(p, g()); }

void bad() {
    f(shared_ptr<int>(new int(2)), g()); }

The function ok follows the guideline to the letter, whereas bad constructs the temporary shared_ptr in place, admitting the possibility of a memory leak. Since function arguments are evaluated in unspecified order, it is possible for new int(2) to be evaluated first, g() second, and we may never get to the shared_ptr constructor if g throws an exception.

The exception safety problem described above may also be eliminated by using the make_shared or allocate_shared factory functions defined in boost/make_shared.hpp. These factory functions also provide an efficiency benefit by consolidating allocations.

Manolo

Answer 2

You need to use:

T* boost::shared_ptr<T>::get()

Example:

BOOST_FOREACH( int i, static_cast< list<int> >( *(GetList().get()) ) ) {

}

The problem is that you can't dereference a boost::shared_ptr and hope it returns the underlying object it stores. If this was true, then there would be no way to dereference a pointer to a boost::shared_ptr. You need to use the specialized ::get() method to return the object stored by boost::shared_ptr, and then dereference that.

See http://www.boost.org/doc/libs/1_46_1/libs/smart_ptr/shared_ptr.htm#get for the documentation.

Pooh

“If this was true, then there would be no way to dereference a pointer to a boost::shared_ptr.” That sounds wrong. There is no logical connection between these two assertions, and in fact it’s at least partially wrong: Dereferencing a shared_ptr does return the underlying object. – Antiperspirant

I no longer get the crash with the line you suggested above. Out of curiosity, I tried BOOST_FOREACH( int i, static_cast< list<int> >(*GetList()) ) as well, which worked, but it's unclear to me why. – Manolo

If I were to guess it's the static_cast is forcing a copy of the list whereas just the *GetList is using a reference to a now destroyed list (because the shared_ptr is destroyed). – Agiotage

Perhaps its nonsense hidden within the FOR_EACH macro that's causing the problems. – Pooh

the static_cast<> is indeed causing the list copy ctor to be called, which I'm trying to avoid have happen in the first place by passing smart pointers around instead of copies of lists – Manolo

you could try this to verify: *static_cast< list<int> * >( GetList().get() ) that is, dereference after the cast instead of before. this would prohibit the copy constructor from running since you are casting be reference now, rather than by value. – Pooh

Yes, I tried this too, but it resulted in the crashing behavior, that is the list gets destructed before BOOST_FOREACH calls begin() on it. – Manolo