c++c++17shared-ptrsmart-pointersunique-ptr

Problem with std::unique_ptr and dangling pointer


When I run this code:

#include <memory>
#include <iostream>
class A {
public:
  virtual std::unique_ptr<A> clone() = 0;
};
class B : public A {
private:
  int b0;
public:
  B(const B& b) { b0 = b.get_b0(); }
  B(const int& b0) : b0(b0) {}
  std:: unique_ptr<A> clone() { return std::make_unique<B>(*this); }
  const int& get_b0() const { return b0; }
  int& get_b0() { return b0; }
};
class box {
private:
  std::unique_ptr<A> o;
public: 
  box(const box& sp) { o = sp.o->clone(); }
  box(std::unique_ptr<A> o)  : o(std::move(o)) {}
  std::unique_ptr<A> get_o() { return o->clone(); }
};
void foo(std::unique_ptr<box> sp) {
  std::unique_ptr<A> o = sp->get_o();
if(const B *ptr1 = dynamic_cast<const B*>(o.get()))
  std::cout << ptr1->get_b0() << std::endl;
if(const B *ptr = dynamic_cast<const B*>(sp->get_o().get()))
  std::cout << ptr->get_b0() << std::endl;
}
int main() {
  B b(100);
  box sp(std::make_unique<B>(b));
  foo(std::make_unique<box>(sp));
  return 0;
}

The output is:

100  
3579964222 (some random integer!!!)

If I ditch std::unique_ptr, and replace it with std::shared_ptr, the code starts working. But in my use case, shared_ptr may lead to problems, as changing one object might affect others.
I like to know how I can fix this code? And if the way I am using clone() is good code practice or not?


Solution

  • It's undefined behavior, ptr is a dangling pointer.

    The expression sp->get_o() produces a temporary object of std::unique_ptr<A>.

    The C++ standard says in class.temporary#4 that:

    Temporary objects are destroyed as the last step in evaluating the full-expression that (lexically) contains the point where they were created.

    And "full-expression" is defined in intro.execution#12.3 as:

    A full-expression is

    • ...
    • an init-declarator or a mem-initializer, including the constituent expressions of the initializer,
    • ...

    Hence, the full-expression here is dynamic_cast<const B*>(sp->get_o().get())), after which is evaluated, the temporary object std::unique_ptr<A> returned by sp->get_o() will be destroyed, and the object it points will be deleted in its destructor. So ptr becomes a dangling pointer.

    To fix it, just extend the lifetime of the object returned by sp->get_o():

    std::unique_ptr<A> o1 = sp->get_o();
    if (const B* ptr = dynamic_cast<const B*>(o1.get())) {
        std::cout << ptr->get_b0() << std::endl;
    }