Boost::shared_ptr instantiated using this

Question

Consider the following case:

typedef boost::shared_ptr<B> BPtr;

class A
{
public:
    A() { b_ptr = BPtr(new B); }
    void a_func() { C::c_func(b_ptr); }
private:
    BPtr b_ptr;
}

class B
{
public:
    B() { b_ptr = BPtr(this); }
    void b_func() { C::c_func(b_ptr); }
private:
    BPtr b_ptr;
}

class C
{
public:
    static void c_func(BPtr b_ptr) { /*...*/ }
}

Is it ok to instantiate a shared_ptr with this?
Is it ok to have two shared_ptr objects pointing to the same object? (e.g. A::b_ptr and B::b_ptr)
If one of these two gets out of scope - would B's instance be deleted?

I'm guessing I'm doing something fundamentally wrong.
I also thought about using dependency injection of b_ptr to B's constructor, but that too seems very wrong.

UPDATE:
To clarify - Both A and B need to use C::c_func. In turn, after some processing, C needs to call a callback function in B which I haven't specified above. Actually two cases are interesting:

If there's a requirement that C won't be stateful - then it needs to receive BPtr both from A and from B as in the code above.
If C is stateful and both A and B instantiate separate C instances, giving a BPtr in C's ctor.

score 10 · Accepted Answer · answered Jan 22 '11 at 00:22

10

Is it ok to instantiate a shared_ptr with this?

No, for at least several reasons:

If you create a A or B object on the stack or statically, you end up with a shared_ptr "owning" the object created on the stack; when the object is destroyed, the shared_ptr destructor will be called, calling delete on the object, and only Bad Things Will Happen. Further, even if the object is created dynamically, you don't know for sure what the owner of your class may do with it: he may assign it to some other type of smart pointer like an auto_ptr, which has totally different semantics.
You end up with a circular reference. For example, b_ptr owns the B object of which it is a member. You have to manually .reset() it in order for the B object to be destroyed.

You can get a shared_ptr from this by using enable_shared_from_this, but there are a lot of caveats: namely, you cannot call enable_shared_from_this in a constructor. Even so, you don't want to store a shared_ptr to an object inside of the object itself; it doesn't make any sense.

I'm guessing I'm doing something fundamentally wrong.

Yes, but since you haven't said what, exactly, you are trying to do, it's hard to say what you should be doing instead. You've only told us how you are trying to do this, not what you are trying to do in the first place.

answered Jan 22 '11 at 00:22

James McNellis

348,265
75
913
977

I marked you up, but I sort of disagree. Saying it isn't OK to instantiate a `shared_ptr` with `this` implies that `enable_shared_from_this` shouldn't exist, and that isn't true. What's wrong here is that it's not OK to have a `shared_ptr` to `this` as a member variable. – Omnifarious Jan 22 '11 at 00:52
@Omnifarious: I think what I said is true: you should never (explicitly, at least) instantiate a new `shared_ptr` object with `this`; doing so would either be horribly wrong or horribly incorrect. You can _implicitly_ do so, using the safe library-provided facility `enable_shared_from_this`. I am all but certain the OP was asking about the explicit form, though. – James McNellis Jan 22 '11 at 02:03
`enable_shared_from_this` requires that one `shared_ptr` be created through "normal" means before you use it, note. – bdonlan Jan 22 '11 at 08:56
@Jonathan: Does `C` actually need to _own_ the `B` object? Does it necessarily have to take a `shared_ptr`? Could it perhaps take a `B&` instead? – James McNellis Jan 22 '11 at 17:34
C just needs to call callbacks in B, I guess a reference would do just fine. I'll try it – Jonathan Livni Jan 23 '11 at 17:11
@James - you were right. I refactored the code to use references and it now makes perfect sense – Jonathan Livni Jan 24 '11 at 15:37
@Jonathan: Good to hear. `shared_ptr` should only be used to express shared ownership, so if something doesn't need ownership of an object, references or raw pointers may be more appropriate (though, it often depends on the specific situation). Feel free to ask any further questions! – James McNellis Jan 24 '11 at 15:55

score 0 · Answer 2 · answered Jan 22 '11 at 00:38

I would recommend that you re-design to use std::enable_shared_from_this, e.g.

class B
{
   B();
public:
   std::shared_ptr<B> create() { return std::make_shared<B>(); } // Make sure B is allocated as shared_ptr inorder to ensure that shared_from_this() works.
};

However, if it's an API issue you have then you can do the following (although I don't recommend it).

void b_func() { C::c_func(BPtr(this, [](B*){})); } // Empty deleter. Nothing happens when shared_ptr goes out of scope. Note that c_func cannot take ownership of the pointer.

Boost::shared_ptr instantiated using this

2 Answers2