1

I have a class Outer which contains an Inner member and owns a vector of unique_ptr elements:

using Elements = std::vector<std::unique_ptr<Element>>;

class Outer
{
    void call()
    {
        _inner.aMethod(_vec);
    }

    Inner _inner;
    Elements _vec;   // This should remain the owner of each Element
};

Inner receives the vector of unique_ptr elements and it transfers ownership to it's own vector class member:

class Inner
{
    public:
    Inner() = default;
    ~Inner() = default;

    void aMethod(Elements& vec)
    {
        _vec = std::move(vec);
    }

    private:

    Elements _vec;  // This is a vector of unique_ptr but I don't want this class to own the memory
};

I stupidly used std::move() because otherwise the compiler complained I was trying to call a deleted function (probably the copy constructor) on each vector element.

I have an illegal memory access and I believe it is because both classes think they own the vector elements and one has tried to delete an already-deleted Element.

How do I have Outer owning the memory and just pass the elements to Inner to use (not take ownership)?

user997112
  • 29,025
  • 43
  • 182
  • 361
  • 2
    std::unique_ptr cannot be duplicated in any way. If it's moved, ownership is passed. Use shared_ptr instead. – Michael Chourdakis Jun 26 '19 at 19:20
  • 4
    Inner could store a reference or a (raw) pointer to the vector in outer rather than its own copy. (As presumably all the Inner instances are own by an Outer, this does not seem risky. Of course, be careful copying Outer objects... – Chris Uzdavinis Jun 26 '19 at 19:20

2 Answers2

3

You cannot have two std::unique_ptrs pointing to the same object. unique_ptr implies unique ownership.

If you need ownership to be shared, use std::shared_ptrs instead. In your example, it should be a drop-in replacement, just change the using declaration:

using Elements = std::vector<std::shared_ptr<Element>>;

If you don't want Inner objects to own the objects pointed to by its _vec member, then it should be a vector of raw pointers:

class Outer
{
    void call()
    {
        std::vector<Element*> observer;
        std::transform(_vec.begin(), _vec.end(), std::back_inserter(observer),
                       [](std::unique_ptr<Element>& el) { return el.get(); });
        _inner.aMethod(observer);
    }

    //...
};

class Inner
{
    // ...

    void aMethod(std::vector<Element*> vec)
    {
        _vec = std::move(vec);
    }

private:
    std::vector<Element*> _vec;
};

Of course, doing this would mean that you run the risk of leaving _vec's elements dangling if Outer frees any of the Elements it owns without updating any Inner objects that are pointing to them. You could partially mitigate that risk by storing a pointer to the Outer object instead of storing pointers directly to the Elements:

class Outer
{
    void call()
    {
        _inner.aMethod(this);
    }

    //...
};

class Inner
{
    // ...

    void aMethod(Outer* outer)
    {
        outer_ = outer;
    }

private:
    Outer* outer_;
};

Inner would then only access its Elements via the Outer object (you could make Inner a friend of Outer if necessary). This still leaves the possibility that an Inner object could outlive the Outer object it points to, but it does reduce the risk somewhat.

Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
1

If Inner is supposed to access the same vector but not own it, why not let Inner keep a reference to the vector in Outer given the fact that they have the same life cycle?

class Inner
{
    public:
    Inner(Elements& vec):_vec(vec) {}
    ~Inner() = default;

    private:

    Elements& _vec;  // This is a reference to vector of unique_ptr, no ownership
};
SPD
  • 370
  • 2
  • 9