BOOST_FOREACH Iteration over boost::shared_ptr

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?

How does it crash? With any assertion fail? Or something else? — Armen Tsirunyan, Jul 04 '11 at 15:51
Unhandled exception at 0x00426692 in BOOSTFOREACH.exe: 0xC0000005: Access violation reading location 0xfeeefeee. — PeskyGnat, Jul 04 '11 at 15:56
I'm assuming the crash is because the list has been destroyed (I traced through the destructor) but BOOST_FOREACH craps out when trying to call list::begin() — PeskyGnat, Jul 04 '11 at 15:57
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. — Sébastien Taylor, Jul 04 '11 at 16:00
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. — PeskyGnat, Jul 04 '11 at 16:02
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. — J T, Jul 04 '11 at 18:01

score 2 · Accepted Answer · edited Jun 20 '20 at 09:12

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.

score 0 · Answer 2 · answered Jul 04 '11 at 16:04

0

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.

answered Jul 04 '11 at 16:04

J T

4,946
5
28
38

1

“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](http://www.boost.org/doc/libs/1_46_1/libs/smart_ptr/shared_ptr.htm#indirection). – Konrad Rudolph Jul 04 '11 at 16:12
I no longer get the crash with the line you suggested above. Out of curiosity, I tried BOOST_FOREACH( int i, static_cast< list >(*GetList()) ) as well, which worked, but it's unclear to me why. – PeskyGnat Jul 04 '11 at 16:35
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). – Sébastien Taylor Jul 04 '11 at 17:00
Perhaps its nonsense hidden within the FOR_EACH macro that's causing the problems. – J T Jul 04 '11 at 18:02
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 – PeskyGnat Jul 04 '11 at 18:11
you could try this to verify: *static_cast< list * >( 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. – J T Jul 04 '11 at 18:17
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. – PeskyGnat Jul 04 '11 at 19:10

BOOST_FOREACH Iteration over boost::shared_ptr

2 Answers2