9

Question for the C++ experts.

We all know that calling shared_from_this() in the class constructor will result in a bad_weak_ptr exception, because no shared_ptr to the instance has been created yet.

As a work-around for that, I came up with this trick:

class MyClass : public std::enable_shared_from_this<MyClass>
{
public:
    MyClass() {}

    MyClass( const MyClass& parent )
    {
        // Create a temporary shared pointer with a null-deleter
        // to prevent the instance from being destroyed when it
        // goes out of scope:
        auto ptr = std::shared_ptr<MyClass>( this, [](MyClass*){} );

        // We can now call shared_from_this() in the constructor:
        parent->addChild( shared_from_this() );
    }

    virtual ~MyClass() {}
};

Someone argued that this is not safe, as the object has not yet been fully formed. Is he right about that?

I'm not using 'this' to access member variables or functions. Furthermore, all member variables have already been initialized, provided I used initializer lists. I don't see how this trick could be unsafe.

Edit: it turns out this trick indeed creates unwanted side-effects. The shared_from_this() will point to the temporary shared_ptr and if you're not careful, the parent-child relationship in my sample code will break. The implementation of enable_shared_from_this() simply does not allow it. Thanks, Sehe, for pointing me in the right direction.

Paul Houx
  • 1,984
  • 1
  • 14
  • 16
  • 1
    Probably handled better with a static factory function calling make_shared and doing the add. No ambiguity about correctness that way. – user4581301 Oct 11 '15 at 20:39
  • One way to blow that sucker up would be `MyClass temp(myparent);` Local variable just got stuffed into a shared pointer. – user4581301 Oct 11 '15 at 20:41
  • That's indeed how I used to handle this (see below), but I was hoping I could make it 'just work'. `static std::shared_ptr create() { /* "initialize" would contain a call to shared_from_this() */ auto ptr = std::make_shared(); ptr->initialize(); return ptr; }` – Paul Houx Oct 12 '15 at 02:00

1 Answers1

6

That's not dangerous.

The documented restriction is: cppreference

Before calling shared_from_this, there should be at least one std::shared_ptr p that owns *this

Nowhere does it say that it can't be used from inside the constructor /for this reason/.

It's just a-typical. That's because under normal circumstances, a make_shared or shared_pointer<T>(new T) cannot complete before the T constructor has exited.

Caveat: the object isn't fully formed so you cannot legally invoke any virtual methods (at the penalty of Undefined Behaviour).

Guideline Since it's possible to use this class wrong (e.g. using shared_ptr<T>(new T) which creates a second shared_ptr with the same underlying pointer value... oops) you should prefer a design that prevents this.

Using a friend factory function that returns the shared_ptr<T> could be one approach.

--> See also The Pit Of Success

sehe
  • 374,641
  • 47
  • 450
  • 633
  • Thanks for your explanation. So if I understand correctly the trick with the temporary `shared_ptr` is not the dangerous part, it's the calling of the `addChild` method if `MyClass` would be deriving from some base class and would have a `MyClass::addChild( const MyClass& child) override` function. What you mention in the Guideline part about having two `shared_ptr`s does not really come into play, as the first one is temporary and does not delete the instance. – Paul Houx Oct 12 '15 at 01:50
  • Yes. It does come into play if the caller doesn't know the constructor "seeds" `shared_from_this`. Better to avoid that possibility. – sehe Oct 12 '15 at 01:52
  • 2
    Now the wording is different: "in particular, `shared_from_this` cannot be called during construction of `*this`" – Mikhail Sep 09 '20 at 10:18