CODEWITHSUNDEEP
c++vectorshared-ptr
Pranav Kapoor
Pranav Kapoor

Reputation: 1281

Shared pointer vector getter in for loop causing problems

I have been experimenting with vectors and shared pointers and I encountered the following scenario. I'm at loss to explain what is happening. The code is 

#include<iostream>
#include<vector>
#include<memory>

class A
{
    public:
    int val;
    A(int val1): val(val1){}
};

class B
{
    std::vector< std::shared_ptr<A> > path;

    public:

    std::vector< std::shared_ptr<A> > getPath() { return path; }

    void doIt()
    {
        std::shared_ptr<A> a1 = std::make_shared<A>(1);
        std::shared_ptr<A> a2 = std::make_shared<A>(2);
        std::shared_ptr<A> a3 = std::make_shared<A>(3);
        path.push_back(a1);
        path.push_back(a2);
        path.push_back(a3);

        std::cout<<"In function"<<std::endl;
        for(std::vector< std::shared_ptr<A> >::iterator itr = path.begin(),
            endItr = path.end(); itr != endItr; ++itr)
        {
            std::cout<<&(*(*itr))<<": "<<(*itr)->val<<std::endl;
        }
    }
};

int main()
{
    B b;
    b.doIt();
    std::cout<<"In main"<<std::endl;
    for(std::vector< std::shared_ptr<A> >::iterator itr = b.getPath().begin(),
    endItr = b.getPath().end(); itr != endItr; ++itr)
    {
        std::cout<<&(*(*itr))<<": "<<(*itr)->val<<std::endl;
    }
}

The output I get is

In function
0x30dc8: 1
0x31780: 2
0x317a0: 3
In main
0x35f18: 196800
0x31780: 2
0x317a0: 3

The 1st element of the vector is for some reason pointing towards another memory location.

Replacing the for loop with the following piece of code solves the problem,

std::vector< std::shared_ptr<A> > path = b.getPath();
for(std::vector< std::shared_ptr<A> >::iterator itr = path.begin(),
    endItr = path.end(); itr != endItr; ++itr)
{
    std::cout<<&(*(*itr))<<": "<<(*itr)->val<<std::endl;
}

Can someone please explain to me what went wrong in the 1st scenario . I'm further interested in knowing why the problem was solved in the second case?

Upvotes: 1

Views: 329

Answers (1)

Barry
Barry

Reputation: 302698

The issue is here:

for(std::vector< std::shared_ptr<A> >::iterator itr = b.getPath().begin(),
    endItr = b.getPath().end(); itr != endItr; ++itr)

getPath() returns a temporary vector. You're calling it twice, so you're getting two different vectors. itr points to the begin() of one temporary vector and endItr points to the end of a different temporary vector. Both temporary vectors go out of scope before the for loop is even entered, so once you dereference you're accessing memory that was already deleted.

Doing this:

std::vector< std::shared_ptr<A> > path = b.getPath();

solves the problem, since now both of your iterators are pointing to the same vector, which will outlive both iterators too.

Also, C++11. You wouldn't have this problem if you just used a range-based for expression:

for (auto& a : b.getPath())
{
    std::cout << &*a << ": " << a->val << std::endl;
}

And that's easier to read.

Upvotes: 4

Related Questions