0

When calling shared_from_this from within types that inherit from enable_shared_from_this, very bad things (TM) can happen, if this is not currently held by a shared_ptr object (typically, the application crashing). Is it possible in C++14 (not 17) to check whether it is safe?

Edit: Withouth using exceptions or try/catch.

curiousguy
  • 8,038
  • 2
  • 40
  • 58
bitmask
  • 32,434
  • 14
  • 99
  • 159
  • 4
    It seems unusual to me to use `enable_shared_from_this` with a type where some objects will be owned by shared pointers and some won't. When using `enable_shared_from_this`, I usually make it impossible to create the object except using public static `create` and `clone` functions. Maybe enforcing that could work with your design and avoid the issue? – aschepler Jun 08 '18 at 10:57
  • They are all held by a `shared_ptr` in the end, but even when using `make_shared`, during the ctor, the object is not yet held by anything. And if that constructor calls a few functions, they might end up trying to access `shared_from_this`. I have the problem in my `invariant` function. – bitmask Jun 08 '18 at 11:10
  • I can now see the hole that you describe. Perhaps the c++17 weak_from_this() method is safer, i.e. could safely return a null weak pointer before the object is make_share()d? – Gem Taylor Jun 08 '18 at 11:23
  • Is the **bad thing** just when calling shared_from_this(), or is it from using that shared pointer without first checking it is not null? If the latter, then shared_from_this() is the function you are looking for! – Gem Taylor Jun 08 '18 at 11:28
  • @GemTaylor: Yes, I am aware of `weak_from_this`, but I'm currently bound to C++14. The "bad thing" is that calling `shared_from_this` is undefined in this situation. But anyway, the backtrace indicates that the throw originates from the call itself. – bitmask Jun 08 '18 at 12:32
  • If it isn't owned, do you have a way out? Or do you just want to put a diagnostic and abort? – curiousguy Jun 08 '18 at 22:54

3 Answers3

1

It is an implementation detail, but could you do something nasty with the internal friend declaration:

template<typename _Tp1>
friend void
__enable_shared_from_this_helper(const __shared_count<>& __pn,
                 const enable_shared_from_this* __pe,
                 const _Tp1* __px) noexcept

Implement your own version with _Tp1 as weak_ptr<>*, that returns the weak pointer [Actually not quite as __px is a const pointer, so you need an extra indirection to lose the const, or if you are being dirty anyway, cast it away!] . Wrap it all in a class that you then derive from instead of enable_shared_from_this:

#if >= C++17
using enable_shared_from_this_c17 = enable_shared_from_this;
#else

template<typename _Tp>
enable_shared_from_this_c17: public enable_shared_from_this<_Tp>
{

  weak_ptr<_Tp> weak_from_this()
  {
    weak_ptr<_Tp> rv; auto rv2 = &rv;
    __enable_shared_from_this_helper(*(const __shared_count<>*)nullptr, this, &rv2);
    return rv;
  }
}
#endif

Now, you have an implementation of weak_from_this() in c++14. Yes, it is a nasty cludge, but it is just until you upgrade to 17.

Alternatively, just catch the exception!

Third alternative - add an instantiate template wrapper that sets "constructed" in a wrapper for enable_shared_from_this that wraps shared_from_this() so it fails until the constructed flag has been set.

enable_shared_from_this
    safe_shared_from_this
        your interfaces
            your_implementation
                constructed<your_implementation>

Of course, this is imperfect if the class is ever used without immediately assigning to a shared_ptr.

Gem Taylor
  • 5,381
  • 1
  • 9
  • 27
  • Ouch, this is certanly a cool hack. But I guess just setting a flag in my postConstruction method or just at the end of the ctor is the easiest. When posting the question, I thought maybe there is a way to check this already in C++14 and I just couldn't find it. – bitmask Jun 08 '18 at 13:50
  • Yes, I guess this is a main reason "they" added weak_from_this – Gem Taylor Jun 08 '18 at 14:10
0

If this is not held by shared_ptr then you cannot do anything. From cppreference: It is permitted to call shared_from_this only on a previously shared object, i.e. on an object managed by std::shared_ptr. Here is example of that:

#include <memory>

class MyClass;

std::shared_ptr<MyClass> global;

void resetGlobal()
{
    global = std::make_shared<MyClass>();
}

class MyClass : public std::enable_shared_from_this<MyClass>
{
public:
    void use()
    {
        resetGlobal();
        shared_from_this();
    }
};

int main()
{
    global = std::make_shared<MyClass>();
    global->use();
    return 0;
}

At the beginning of use method shared_from_this should be fine (except in multithread environment without mutexes) but after calling any function that can alter shared_ptrs elsewhere you cannot be sure. After calling resetGlobal function this is simply destroyed and any access to it is undefined behavior so could result in segmentation fault.

The only way to be sure that through entire method call shared_from_this is valid is to make temporary shared_ptr at the beginning of call:

void use()
{
    std::shared_ptr<MyClass> tmp = shared_from_this();
    resetGlobal();
    shared_from_this();
}

or

{
    std::shared_ptr<MyClass> tmp = global;
    global->use();
}
Mateusz Drost
  • 1,171
  • 10
  • 23
0

If any other object in your program has access to your object's raw this pointer, then you are using shared_ptr entirely the wrong way.

Any pointer to your object to be used externally to your object MUST be wrapped in a shared_ptr<YourObject> value instance, no exceptions.

joeybladb
  • 227
  • 1
  • 10
  • _"Any pointer to your object ... MUST be wrapped in a shared_ptr value instance"_ I don't agree with that. A `shared_ptr` is more costly, but that cost is justified if it may need to change object lifetime. – Drew Dormann Feb 24 '20 at 16:44