static_pointer_cast and enable_shared_from_this

[Johannes Sixt]

Consider this program:

#include <memory>
using namespace std;

struct Base {
virtual ~Base() = default;
};

struct Derived : Base, enable_shared_from_this<Derived> {
};

int main()
{
Base* raw = new Derived;
shared_ptr<Base> b(raw);
shared_ptr<Derived> d = static_pointer_cast<Derived>(b);
d->shared_from_this(); // GCC and MSVC throw bad_weak_ptr
}

That is:
- the Base class is not shared_from_this-enabled
- but the Derive class is
- the object was first stored in a shared_ptr<Base>
- but then propagated to a shared_ptr<Derived>

Is the behavior that a bad_weak_ptr exception is thrown covered by the
Standard?

The note at the end of 20.8.2.5 [util.smartptr.enab] about a possible
implemention mentions: "The shared_ptr constructors that create unique
pointers can detect the presence of an enable_shared_from_this base..."
If only such constructors need to take action, the observed behavior
would not be a surprise. But the note is not normative.

I cannot conclude from the specification of enable_shared_from_this that
only the unique pointer constructors need to take action. On the
contrary, the specification of shared_from_this() is so broad that all
shared_ptrs owning a pointer to an enable_shared_from_this derived class
must be able to handle shared_from_this() without throwing an exception,
in particular also in the situation illustrated by the program above.

Am I expecting too much?

-- Hannes

[Ville Voutilainen]

On 28 July 2015 at 15:46, Johannes Sixt <j...@kdbg.org> wrote:
> Consider this program:
>
> #include <memory>
> using namespace std;
>
> struct Base {
> virtual ~Base() = default;
> };
>
> struct Derived : Base, enable_shared_from_this<Derived> {
> };
>
> int main()
> {
> Base* raw = new Derived;
> shared_ptr<Base> b(raw);
> shared_ptr<Derived> d = static_pointer_cast<Derived>(b);
> d->shared_from_this(); // GCC and MSVC throw bad_weak_ptr
> }
>
> That is:
> - the Base class is not shared_from_this-enabled
> - but the Derive class is
> - the object was first stored in a shared_ptr<Base>

And at that point there was no chance to detect a enable_shared_from_this base.
That detection isn't dynamic, as far as I understand.

> - but then propagated to a shared_ptr<Derived>

At which point we no longer know that there was an
enable_shared_from_this available
originally.

> Is the behavior that a bad_weak_ptr exception is thrown covered by the
> Standard?

I certainly hope so.

> Am I expecting too much?

I think you are. :)

[Johannes Sixt]

Am 28.07.2015 um 14:57 schrieb Ville Voutilainen:
> On 28 July 2015 at 15:46, Johannes Sixt <j...@kdbg.org> wrote:
>> Consider this program:
>>
>> #include <memory>
>> using namespace std;
>>
>> struct Base {
>> virtual ~Base() = default;
>> };
>>
>> struct Derived : Base, enable_shared_from_this<Derived> {
>> };
>>
>> int main()
>> {
>> Base* raw = new Derived;
>> shared_ptr<Base> b(raw);
>> shared_ptr<Derived> d = static_pointer_cast<Derived>(b);
>> d->shared_from_this(); // GCC and MSVC throw bad_weak_ptr
>> }
>>
>> That is:
>> - the Base class is not shared_from_this-enabled
>> - but the Derive class is
>> - the object was first stored in a shared_ptr<Base>
>
> And at that point there was no chance to detect a enable_shared_from_this base.
> That detection isn't dynamic, as far as I understand.

Incidentally,

shared_ptr<Base> p(new Derived);

does detect enable_shared_from_this (I tested only with GCC).

>> - but then propagated to a shared_ptr<Derived>
>
> At which point we no longer know that there was an
> enable_shared_from_this available
> originally.

It would certainly be possible for static_pointer_cast and
dynamic_pointer_cast to detect enable_shared_from_this in the derived class.

-- Hannes

[Ville Voutilainen]

On 28 July 2015 at 16:11, Johannes Sixt <j...@kdbg.org> wrote:
>>> Base* raw = new Derived;
>>> shared_ptr<Base> b(raw);

>> And at that point there was no chance to detect a enable_shared_from_this
>> base.
>> That detection isn't dynamic, as far as I understand.
> Incidentally,
> shared_ptr<Base> p(new Derived);
> does detect enable_shared_from_this (I tested only with GCC).

Yes, because the constructor can take any pointer, not just a Base*.
However, in your
original snippet, you already converted a Derived* to Base* before
giving it to shared_ptr,
and then the knowledge of a enable_shared_from_this base class was lost. In the
case of constructing a shared_ptr<Base> from a Derived*, that
knowledge is not lost.

>>> - but then propagated to a shared_ptr<Derived>
>> At which point we no longer know that there was an
>> enable_shared_from_this available
>> originally.
> It would certainly be possible for static_pointer_cast and
> dynamic_pointer_cast to detect enable_shared_from_this in the derived class.

But that doesn't help, because the control block would not be in the correct
state, since we weren't able to fiddle with the refcounts of the non-existent
enable_shared_from_this in the original shared_ptr<Base>. Havoc ensues,
likely in the form of double deletes. bad_weak_ptr is a much more benign
response.

[Johannes Sixt]

Am 28.07.2015 um 15:18 schrieb Ville Voutilainen:
> On 28 July 2015 at 16:11, Johannes Sixt <j...@kdbg.org> wrote:
>>>> Base* raw = new Derived;
>>>> shared_ptr<Base> b(raw);
>>> And at that point there was no chance to detect a enable_shared_from_this
>>> base.
>>> That detection isn't dynamic, as far as I understand.
>> Incidentally,
>> shared_ptr<Base> p(new Derived);
>> does detect enable_shared_from_this (I tested only with GCC).
>
> Yes, because the constructor can take any pointer, not just a Base*.
> However, in your
> original snippet, you already converted a Derived* to Base* before
> giving it to shared_ptr,
> and then the knowledge of a enable_shared_from_this base class was lost. In the
> case of constructing a shared_ptr<Base> from a Derived*, that
> knowledge is not lost.

Of course. I know.

>>>> - but then propagated to a shared_ptr<Derived>
>>> At which point we no longer know that there was an
>>> enable_shared_from_this available
>>> originally.
>> It would certainly be possible for static_pointer_cast and
>> dynamic_pointer_cast to detect enable_shared_from_this in the derived class.
>
> But that doesn't help, because the control block would not be in the correct
> state, since we weren't able to fiddle with the refcounts of the non-existent
> enable_shared_from_this in the original shared_ptr<Base>. Havoc ensues,
> likely in the form of double deletes. bad_weak_ptr is a much more benign
> response.

I do not share your concerns. Filling in the weak_ptr stored in
enable_shared_from_this is just a matter of assigning a shared_ptr to a
weak_ptr. How could this cause chaos? This should be easily doable by
static_pointer_cast and dynamic_pointer_cast.

BTW, I am more interested in an answer to the question whether the
observed behavior is covered by the Standard, because from my reading I
conclude that it is not.

-- Hannes

[Ville Voutilainen]

On 28 July 2015 at 16:46, Johannes Sixt <j...@kdbg.org> wrote:
>> But that doesn't help, because the control block would not be in the
>> correct
>> state, since we weren't able to fiddle with the refcounts of the
>> non-existent
>> enable_shared_from_this in the original shared_ptr<Base>. Havoc ensues,
>> likely in the form of double deletes. bad_weak_ptr is a much more benign
>> response.
> I do not share your concerns. Filling in the weak_ptr stored in
> enable_shared_from_this is just a matter of assigning a shared_ptr to a
> weak_ptr. How could this cause chaos? This should be easily doable by
> static_pointer_cast and dynamic_pointer_cast.

Right, it probably causes no chaos, but you're suggesting that
shared_ptr checks whether an enable_shared_from_this of its target
object has a null weak_ptr in it, and if so, assigns itself to it?
I suppose that works as far as refcounts go, and I can't immediately
see any chaos in it, it's
merely an additional check that's meant to protect from cases that are
not supposed to happen. :)

With regards to whether the behavior is covered by the standard, I'd
say it is not spelled
out very clearly. The whole business of how enable_shared_from_this
works is not very
clearly specified imho.

[Thiago Macieira]

On Tuesday 28 July 2015 17:18:22 Ville Voutilainen wrote:
> Right, it probably causes no chaos, but you're suggesting that
> shared_ptr checks whether an enable_shared_from_this of its target
> object has a null weak_ptr in it, and if so, assigns itself to it?
> I suppose that works as far as refcounts go, and I can't immediately
> see any chaos in it, it's
> merely an additional check that's meant to protect from cases that are
> not supposed to happen.

There's a race here, as the weak_ptr may have been shared to multiple threads,
which may be simultaneously trying to create a shared_ptr.

It's not impossible to do it, but it needs to be done carefully.

Another thing to think about is when the enable_shared_from_this comes from a
different side of the hierarchy:

struct Base {
virtual ~Base() = default;
};

struct Base2 : enable_shared_from_this<Derived> {
virtual ~Base2() = default;
};

struct Derived : Base, Base2 {
};

Does it still apply?

--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
PGP/GPG: 0x6EF45358; fingerprint:
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358