Why does std::enable_shared_from_this allow multiple std::shared_ptr instances?

Question

There are several questions that cover the behavior of std::enable_shared_from_this, but I don't think that this is a duplicate.

Classes that inherit from std::enable_shared_from_this carry a std::weak_ptr member. When the application creates a std::shared_ptr pointing to a subclass of std::enable_shared_from_this, the std::shared_ptr constructor checks the std::weak_ptr, and if it is not initialized, initializes it and uses the std::weak_ptr control block for the std::shared_ptr. However, if the std::weak_ptr is already initialized, the constructor just creates a new std::shared_ptr with a new control block. This sets the application up to crash when the reference count of one of the two std::shared_ptr instances goes to zero and deletes the underlying object.

struct C : std::enable_shared_from_this<C> {};

C *p = new C();
std::shared_ptr<C> p1(p);

// Okay, p1 and p2 both have ref count = 2
std::shared_ptr<C> p2 = p->shared_from_this();

// Bad: p3 has ref count 1, and C will be deleted twice
std::shared_ptr<C> p3(p);

My question is: why does the library behave this way? If the std::shared_ptr constructor knows that the object is a std::enable_shared_from_this subclass and bothers to check the std::weak_ptr field, why doesn't it always use same control block for the new std::shared_ptr, thus avoiding a potential crash?

And for that matter, why does the method shared_from_this fail when the std::weak_ptr member is not initialized, instead of just initializing it and returning a std::shared_ptr?

It seems strange that the library works the way that it does, since it fails in situations when it could easily succeed. I'm wondering if there were design considerations/limitations that I don't understand.

I am using Clang 8.0.0 in C++17 mode.

For the second question, initializing a new `std::shared_ptr` instead of failing on `shared_from_this`, you can't assume how the object's lifetime is currently being managed. Unless it's owned by a raw pointer that the user won't `delete`, there really isn't a scenario where this will lead to anything good. For example, what happens if I tried `shared_from_this` with automatic storage duration? — François Andrieux, Jun 05 '19 at 17:46
`the std::shared_ptr constructor checks the std::weak_ptr, and if it is not initialized, initializes it and uses the std::weak_ptr control block for the std::shared_ptr` I don't think this is accurate. [cppreference](https://en.cppreference.com/w/cpp/memory/enable_shared_from_this/enable_shared_from_this) states that the `weak_ptr` is value initialized. I would assume that means no check is made. So adding the check would add cost to every construction that does not require it. — super, Jun 05 '19 at 18:45
@FrançoisAndrieux fair enough on the point about the object not assuming that `this` is a pointer from which a shared pointer could/should be created. — Willis Blackburn, Jun 05 '19 at 21:11
Having multiple smart pointers to the "same object" (whatever that means) isn't inherently a bad idea. — curiousguy, Jun 07 '19 at 00:39

score 5 · Accepted Answer · answered Jun 05 '19 at 19:14

If I understand your question correctly, you would assume that calling the constructor shared_ptr a second time would logically reuse the control block stored in shared_from_this.

From your point of view, this looks logical. Let's assume for a moment that C is part of a library your are maintaining and the usage of C is part of a user of your library.

struct C : std::enable_shared_from_this<C> {};

C *p = new C();
std::shared_ptr<C> p1(p);
std::shared_ptr<C> p3(p); // Valid given your assumption

Now, you found a way to no longer need the enable_shared_from_this and in the next version of your library, this gets updated to:

struct C {};

C *p = new C();
std::shared_ptr<C> p1(p);
std::shared_ptr<C> p3(p); // Now a bug

Suddenly, perfectly valid code becomes invalid without any compiler error/warning, because of upgrading your library. Where possible this should be prevented.

At the same time, it will cause a lot of confusion. Cause depending on the implementation of the class you put in shared_ptr, it's either defined or undefined behavior. It's less confusing to make it undefined every single time.

enable_shared_from_this is a standard workaround for getting a hold of a shared_ptr if you don't have a shared_ptr. A classic example:

 struct C : std::enable_shared_from_this<C>
 {
     auto func()
     {
         return std::thread{[c = this->shared_from_this()]{ /*Do something*/ }};
     }

     NonCopyable nc;
 };

Adding your mentioned extra functionality does add extra code whenever you don't need it, just for the check. Not that it would matter that much, though, zero overhead abstractions ain't nearly zero overhead abstractions.

Thanks, this is pretty clear. You're saying that creating two `std::shared_ptr` instances from the same raw pointer is normally bad/undefined behavior, so it shouldn't suddenly start working just because the underlying object happens to be of some magic class. Makes sense. — Willis Blackburn, Jun 05 '19 at 21:20
Yes, indeed. Which on it's own isn't a big issue, though, it becomes a headache if you need to maintain a big code base where one of the base classes changes semantics. — JVApen, Jun 05 '19 at 21:26
Many small changes can silently break that uses `enable_shared_from_this` notably changing to private inheritance (for what many ppl believe is an implementation detail), change in stdlib impl if you made a stdlib function friend to fix the previous issue, turning an object (created with `make_shared`) into a member subobject... — curiousguy, Jun 07 '19 at 00:43
@curiousguy Exactly my point. Making the standard library friend ain't a good idea, so that's one of the cases I'm least afraid of — JVApen, Jun 07 '19 at 07:03

score 0 · Answer 2 · answered Jun 07 '19 at 13:29

0

This is not an answer to the question but more of a reply to user jvapen based on his answer to this question.

You had stated this in your answer:

struct C {};

C *p = new C();
std::shared_ptr<C> p1(p);
std::shared_ptr<C> p3(p); // Now a bug

What I'm not seeing here is how line 5 std::shared_ptr<C> p3(p); is now a bug. According to cppreference:shared_ptr they specifically state this:

std::shared_ptr is a smart pointer that retains shared ownership of an object through a pointer. Several shared_ptr objects may own the same object.

answered Jun 07 '19 at 13:29

Francis Cugler

7,788
2
28
59

3

If you keep reading you'll also see this: "The ownership of an object can only be shared with another shared_ptr by copy constructing or copy assigning its value to another shared_ptr. Constructing a new shared_ptr using the raw underlying pointer owned by another shared_ptr leads to undefined behavior." – Willis Blackburn Jun 08 '19 at 01:34
@WillisBlackburn Thank you needed some clarification on that! – Francis Cugler Jun 08 '19 at 02:50
1

To weight in: you can only transfer ownership once. This would be more obvious when using `unique_ptr` for ownership, as you have to move it. That's why the general consensus is to replace new/delete with `make_unique` whenever possible. – JVApen Jun 09 '19 at 21:00

score -1 · Answer 3 · answered Jun 05 '19 at 17:55

-1

Creating two shared_ptrs to the same pointer is undefined behaviour and is nothing to do with std::enable_shared_from_this. Your code should be:

struct C : std::enable_shared_from_this<C> {};

C *p = new C();
std::shared_ptr<C> p1(p);

std::shared_ptr<C> p2 = p->shared_from_this();

std::shared_ptr<C> p3(p1);

answered Jun 05 '19 at 17:55

Alan Birtles

32,622
4
31
60

3

The question is why the language doesn't have a feature where OP's code would work, based on `C` knowing it already has an owning `std::shared_ptr`. OP clearly communicated that they know their code is problematic. – François Andrieux Jun 05 '19 at 17:56
@FrançoisAndrieux: "*OP clearly communicated that they know their code is problematic.*" I disagree. You don't generally ask why your problematic code isn't made unproblematic. The existence of the question suggests, at least, that the OP doesn't think their code *should be* particularly problematic. – Nicol Bolas Jun 05 '19 at 18:25
@FrançoisAndrieux: But... the code *is bad*. That is the answer: it's not allowed because a user doing this is doing the wrong thing, so we don't let users do the wrong thing. Even though it *could* be "fixed", to "fix" it would be to say that the incoherent code is not incoherent. – Nicol Bolas Jun 05 '19 at 18:33
4

@NicolBolas The question is *why* does `enable_shared_from_this` behave this way. The fact that the code is bad, and why, is explained in the question. Re-stating that is hardly an answer. – super Jun 05 '19 at 18:36
@FrançoisAndrieux "*An analogue would be answering the question "I dropped my phone and it broke, so why don't companies make tougher phones?"*" I would say that a better analog is "I put my phone in front of my car tires and accelerated over it, and it broke. Why don't companies make phones that you can drive over?" The point of the answer here is that you're not *supposed* to be able to do these things. They're not allowed because they're not *supposed* to be allowed. – Nicol Bolas Jun 05 '19 at 18:54
@FrançoisAndrieux: "*Maybe nobody though of it, maybe the solution doesn't account for a technical limitation, maybe it's a trade off with something else that's more important.*" Or maybe it has nothing to do with technical limitations and has more to do with "we don't want to facilitate misuse of the API". Which is the point being made here: you aren't supposed to be able to do it. – Nicol Bolas Jun 05 '19 at 19:08
@NicolBolas Okay, I see what you mean and it makes sense. That might be a good answer, but it isn't what this answer provides. Edit : If that *is* what the answer is trying to communicate, even rereading it with this interpretation in mind, I don't see it. – François Andrieux Jun 05 '19 at 19:08
@NicolBolas you're explaining how to write code that will work given the way the standard library works today. I understand that already. I'm asking why the library works that way, instead of an arguably better way, which I describe in the question. – Willis Blackburn Jun 05 '19 at 21:14
"_Creating two shared_ptrs to the same pointer is undefined behaviour_" Since when? – curiousguy Jun 07 '19 at 00:43

score -1 · Answer 4 · answered Jun 09 '19 at 05:08

-1

Creating a secondary smart pointer that is either actually non owning (it does nothing when the last copy is reset/destroyed), or carries a copy of the original smart pointer in the control block (in the deleter object) such that when the secondary reference count goes to zero the primary refcount is decremented, is a very rare occurrence and would probably be confusing for most programmers, but it isn't inherently illegal. (And I think one can make a strong case for that pattern in special cases.)

On the other hand, the existence shared_from_this strongly suggests that there is only one owning shared_ptr, and so having shared_from_this should probably be avoided when multiple sets of std::shared_ptr are expected. Explicit management of self referential non owning pointers is safer as it makes such issues evident in user code, unlike the implicit behavior of std::enable_shared_from_this.

answered Jun 09 '19 at 05:08

curiousguy

8,038
2
40
58

"And I think one can make a strong case for that pattern in special cases." Could you give some examples? – Willis Blackburn Jun 10 '19 at 13:15
A simple example is when an object is never destroyed (or not before program termination) and you want to create non owning smart pointers to it; creating multiple independent `shared_ptr` obviously is not worse than creating just one. – curiousguy Jun 10 '19 at 16:17
The 1st para of the answer contains some actually: "_smart pointer that is either actually non owning (it does nothing when the last copy is reset/destroyed), or carries a copy of the original smart pointer in the control block (in the deleter object) such that when the secondary reference count goes to zero the primary refcount is decremented_" (Now you can find these ideas downright *bizarre* and even believe they are anti-patterns. It's subjective.) – curiousguy Jun 10 '19 at 18:50
I was looking for examples of code that uses shared-pointers-to-shared-pointers or "non-owning" shared pointers and some explanation of why such uses are necessary. You suggested that such uses could be confusing and/or bizarre but that a strong case could me made for using them. I'm wondering what that strong case would be. – Willis Blackburn Jun 13 '19 at 12:49
@WillisBlackburn Non owning shared owning pointer is needed when you use a library component that takes a `const shared_ptr&` (or `shared_ptr`) and copies it in order **to extend the lifetime of the managed object**. A non owning object of course cannot extend the lifetime of anything but for a global object that is never destroyed (or only at program termination) that will be OK. You tell the library that it can extend the lifetime of an object when it really can't, but it doesn't matter. – curiousguy Jun 13 '19 at 14:35
@WillisBlackburn If you have many threads using a common shared object (mostly read only) you might want to avoid the RMW atomic operation on a common shared object; you could use secondary `shared_ptr` that hold a primary `shared_ptr` so that instead of one refcount object you have many different objects and less ping pong occur between cores. Of course at the end of the day more RMW have to be performed as there are more atomic objects but with less sharing, more locality. I have no tested and I don't know any real world use and practical test of that idea. – curiousguy Jun 13 '19 at 23:39

Why does std::enable_shared_from_this allow multiple std::shared_ptr instances?

4 Answers4