Emiliano
Emiliano

Reputation: 23732

shared_from_this called from constructor

I have to register an object in a container upon its creation. Without smart pointers I'd use something like this:

a_class::a_class()
{
    register_somewhere(this);
}

With smart pointers I should use shared_from_this but I can't use that in the constructor.

Is there a clean way to solve this problem? What would you do in a similar situation? I'm thinking about introducing an init method to call just after creation and put everything in a factory function like this:

boost::shared_ptr<a_class> create_a()
{
    boost::shared_ptr<a_class> ptr(new a_class);
    ptr->init();
    return ptr;
}

Is it fine or there is a standard procedure to follow in such cases?

EDIT: Actually my case is more complex. I have 2 object which shall maintain pointers each other. So the truth is I'm not "registering" but creating another object (let's say b_class) which requires this as a parameter. b_class receives this as a weak pointer and stores it.

I'm adding this because since you are giving me design advices (which are very appreciated) at least you can know what I'm doing:

a_class::a_class()
{
    b = new b_class(this);
}

In my program a_class is an entity and b_class is one of the concrete classes representing the state (in the constructor it's just the starting state). a_class needs a pointer to the current state and b_class needs to manipulate the entity.

a_class is responsible for creating and destroying b_class instances and thus maintains a shared_ptr to them but b_class need to manipulate a_class and thus maintains a weak pointer. a_class instance "survives" b_class instances.

Do you suggest to avoid using smart pointers in this case?

Upvotes: 32

Views: 15735

Answers (7)

Andrew
Andrew

Reputation: 1

Here's my solution:

class MyClass: enable_shared_from_this<MyClass>
{
    public:
        //If you use this, you will die.
        MyClass(bool areYouBeingAnIdiot = true)
        {
            if (areYouBeingAnIdiot)
            {
                throw exception("Don't call this constructor! Use the Create function!");
            }

            //Can't/Don't use this or shared_from_this() here.
        }

        static shared_ptr<MyClass> Create()
        {
            shared_ptr<MyClass> myClass = make_shared<MyClass>(false);

            //Use myClass or myClass.get() here, now that it is created.

            return myClass;
        }
}

//Somewhere.
shared_ptr<MyClass> myClass = MyClass::Create();

(The constructor has to be public to be callable from a static member function, even an internal one...)

Upvotes: 0

Kijewski
Kijewski

Reputation: 26022

I came up with a helper class for that problem:

template <class Impl>
class ImmediatelySharedFromThis : public std::enable_shared_from_this<Impl> {
    typedef std::unique_ptr<void, std::function<void(void*)>> MallocGuard;
    typedef std::shared_ptr<Impl> SharedPtr;

    // disallow `new MyClass(...)`
    static void *operator new(size_t) = delete;
    static void *operator new[](size_t) = delete;
    static void operator delete[](void*) = delete;
protected:
    typedef std::pair<MallocGuard&, SharedPtr&> SharingCookie;

    ImmediatelySharedFromThis(SharingCookie cookie) {
        MallocGuard &ptr = cookie.first;
        SharedPtr &shared = cookie.second;
        // This single line contains the actual logic:
        shared.reset(reinterpret_cast<Impl*>(ptr.release()));
    }
public:
    // Create new instance and return a shared pointer to it.
    template <class ...Args>
    static SharedPtr create(Args &&...args) {
        // Make sure that the memory is free'd if ImmediatelySharedFromThis
        // is not the first base class, and the initialization
        // of another base class throws an exception.
        MallocGuard ptr(aligned_alloc(alignof(Impl), sizeof(Impl)), free);
        if (!ptr) {
            throw std::runtime_error("OOM");
        }

        SharedPtr result;
        ::new (ptr.get()) Impl(SharingCookie(ptr, result),
                               std::forward<Args>(args)...);
        return result;
    }

    static void operator delete(void *ptr) {
        free(ptr);
    }
};

class MyClass : public ImmediatelySharedFromThis<MyClass> {
    friend class ImmediatelySharedFromThis<MyClass>;

    MyClass(SharingCookie cookie, int some, int arguments) :
        ImmediatelySharedFromThis(cookie)
        // You can pass shared_from_this() to other base classes
    {
        // and you can use shared_from_this() in here, too.
    }
public:
    ....
};

...

std::shared_ptr<MyClass> obj = MyClass::create(47, 11); 

A bit ugly, but it works.

Upvotes: 1

Walter
Walter

Reputation: 45434

There is no need for shared_ptr in your code (as you show it and explain it). A shared_ptr is only appropriate for shared ownership.

Your b_class doesn't own it's a_class, in fact is even outlived by it, so it should merely keep an observing pointer.

If b_class is polymorphic and the manipulations of a_class involve altering its b_class pointer, you should use a unique_ptr<b_class>:

class a_class;
class b_class
{
  friend class a_class;
  a_class* mya;
  b_class(a_class*p)
  : mya(p) {}
public:
  virtual~b_class() {}   // required for unique_ptr<b_class> to work
  virtual void fiddle(); // do something to mya
};

class a_class
{
  std::unique_ptr<b_class> myb;
public:
  a_class()
  : myb(new b_class(this)) {}
  template<typename B>
  void change_myb()
  {
    myb.reset(new B(this));
  }
};

Upvotes: 1

Jakob Riedle
Jakob Riedle

Reputation: 2018

For this purpose, I wrote my own drop-in replacement for shared_ptr, weak_ptr and enable_shared_from_this. You can check it out at Sourceforge.

It allows call of shared_from_this() in constructor and destructor without helper functions without space overhead compared to the standard enable_shared_from_this.

Note: creating shared_ptr's in dtors is only allowed as long as they are created only temporarily. That is, they are being destroyed before the dtor returns.

Upvotes: 0

CB Bailey
CB Bailey

Reputation: 791909

a_class is responsible for creating and destroying b_class instances

...

a_class instance "survives" b_class instances.

Given these two facts, there should be no danger that a b_class instance can attempt to access an a_class instance after the a_class instance has been destroyed as the a_class instance is responsible for destroying the b_class instances.

b_class can just hold a pointer to it's associated a_class instance. A raw pointer doesn't express any ownership which is appropriate for this case.

In this example it doesn't matter how the a_class is created, dynamically, part of a aggregated object, etc. Whatever creates a_class manages its lifetime just as a_class manages the lifetime of the b_class which it instantiates.

E.g.

class a_class;

class b_class
{
public:
    b_class( a_class* a_ ) : a( a_ ) {}
private:
    a_class* a;
};

class a_class
{
public:
    a_class() : b( new b_class(this) ) {}
private:
    boost::shared_ptr<b_class> b;
};

Note, in this toy example there is no need for a shared_ptr, an object member would work just as well (assuming that you don't copy your entity class).

class a_class
{
public:
    a_class() : b( this ) {}
private:
    b_class b;
};

Upvotes: 8

Maxim Egorushkin
Maxim Egorushkin

Reputation: 136266

Why don't you use http://www.boost.org/doc/libs/1_43_0/libs/smart_ptr/enable_shared_from_this.html

struct a_class : enable_shared_from_this<a_class> {
    a_class() {
        shared_ptr<a_class> ptr(this);
        register_somewhere(ptr);
    }
};

Update: here is a complete working example:

#include <stdio.h>
#include <boost/smart_ptr/enable_shared_from_this.hpp>

struct a_class;
boost::shared_ptr<a_class> pa;

void register_somewhere(boost::shared_ptr<a_class> p)
{
    pa = p;
};

struct a_class : boost::enable_shared_from_this<a_class> {
private:
    a_class() {
        printf("%s\n", __PRETTY_FUNCTION__);
        boost::shared_ptr<a_class> ptr(this);
        register_somewhere(ptr);
    }

public:
    ~a_class() {
        printf("%s\n", __PRETTY_FUNCTION__);
    }

    static boost::shared_ptr<a_class> create()
    {
        return (new a_class)->shared_from_this();
    }
};

int main()
{
    boost::shared_ptr<a_class> p(a_class::create());
}

Note the factory function a_class::create(). Its job is to make sure that only one reference counter gets created. Because

boost::shared_ptr<a_class> p(new a_class);

Results in creation of two reference counters and double deletion of the object.

Upvotes: 1

Collin Dauphinee
Collin Dauphinee

Reputation: 13973

If you absolutely need a shared_ptr during construction, it's best to have an 'init' function. In fact, this is the only decent approach I can think of. You should probably have a special function that creates objects of this type, to ensure init() is called, if you choose this path.

However, depending on what you're registering for, it may be a better idea to give whatever object you're registering with a plain pointer to the object in the constructor, rather than a shared_ptr. Then in the destructor, you can just unregister the object from the manager.

Upvotes: 5

Related Questions