Search code examples
c++c++11smart-pointers

Initialize field with raw pointer to object after base class constructed with moved unique_ptr to same object


I have an object Foo owned by Owner and no one else. I create an object of class Derived that inherits Owner, passing it a unique_ptr to a newly created Foo. Derived has a member of class User that also needs to use Foo, but doesn't own it, so it gets only a raw pointer to it.

#include <memory>
#include <iostream>
using namespace std;

struct Foo {
  void talk() { cout << "foo" << endl; }
};

struct User {
  Foo *foo;
  User(Foo *_foo): foo(_foo) {};
  void use() { foo->talk(); } //potential disaster if foo's memory has changed
};

struct Owner {
  unique_ptr<Foo> foo;
  Owner(unique_ptr<Foo> _foo): foo(move(_foo)) {};
};

struct Derived : public Owner {
  User user;
  Derived(unique_ptr<Foo> _foo)
  : Owner {move(_foo)},
    user{_foo.get()}  //potential disaster: _foo has already been move()'d, can't get()!
  {};
  void use() { user.use(); };
};

int main() {
  Derived d(make_unique<Foo>());
  //do other stuff
  d.use();  //potential disaster
}

The problem is that in the initialization list of Derived, I need to move() the Foo to pass it to Owner's constructor (which takes a unique_ptr<Foo>, because it is the owner), and also pass a raw pointer to the Foo to User. But by the time the User gets initialized, the unique_ptr<Foo> has already been moved and foo.get() may point to an invalid location!

What's a good way to make sure User gets a valid (non-owning) pointer to Foo and Owner still gets its unique_ptr<Foo>?


Solution

  • You could delegate the work to a private constructor while duplicating the pointer:

    struct Derived : public Owner {
      User user;
      Derived(unique_ptr<Foo> _foo)
      : Derived(move(_foo), _foo.get())
      {};
    
      void use() { user.use(); };
    private:
      // Implement the real logic here
      // As pointed by @Artyer in the comments, this r-value ref is required. 
      // Pass by value would again introduce UB in the calling code.
      Derived(unique_ptr<Foo>&& _foo, Foo* ptr)
       : Owner {move(_foo)},
        user{ptr} {}
    };
    

    Note, that Derived(move(_foo),_foo.get()) is safe even though the order of evaluation is unspecified because std::move is just a cast.

    Just for clarity, this is not a potential disaster but a certain one since the initializer list is evaluated from left to right.

    9.4.4.4 [dcl.init.list]:

    Within the initializer-list of a braced-init-list, the initializer-clauses, including any that result from pack expansions ([temp.variadic]), are evaluated in the order in which they appear. That is, every value computation and side effect associated with a given initializer-clause is sequenced before every value computation and side effect associated with any initializer-clause that follows it in the comma-separated list of the initializer-list. [Note: This evaluation ordering holds regardless of the semantics of the initialization; for example, it applies when the elements of the initializer-list are interpreted as arguments of a constructor call, even though ordinarily there are no sequencing constraints on the arguments of a call. — end note] .

    ADDED: Why does the private constructor require pass by reference

    @Artyer posted the correct explanation with a counter-example but since then has unfortunately deleted it.

    There are two reasonable ways to write the delegating constructor:

    Derived(unique_ptr<Foo>&& _foo, Foo* ptr) //(A)
    Derived(unique_ptr<Foo> _foo, Foo* ptr) //(B) Since the move is cheap.
    

    And suppose that it was called like this:

    Derived(move(_foo), _foo.get()) //(1)
    Derived{move(_foo), _foo.get()} //(2) @Artyer's idea
    

    First, by adding a delegating constructor we essentially added a sequencing point that had to evaluate both the unique and raw pointer before passing them to Owner and User, respectively.

    Second, all the arguments for a function call must be evaluated before the function is actually called, this order is unspecified for () and left-to-right for {}(see the delete paragraph above).

    Here lies the difference, if (B) is used, the copy has to be made at the call place and for () this copy can be made before assigning _foo.get() to ptr argument. For {} it will always happen before. Thus setting ptr incorrectly.

    On the other hand, if (A) is chosen, only the r-value reference is copied into the argument and ptr will always get the correct value no matter whether (1) or (2) is used.