Issue with shared_from_this after utilizing load_and_allocate

[Devacor]

This stumped me for some time. I was looking for errors in my own code for quite some time before I created the following example. enable_shared_from_this doesn't seem to get its weak_ptr reconstituted properly on load.

#include <strstream>

#include "cereal/cereal.hpp"
#include "cereal/types/map.hpp"
#include "cereal/types/vector.hpp"
#include "cereal/types/memory.hpp"
#include "cereal/types/string.hpp"
#include "cereal/types/base_class.hpp"

#include "cereal/archives/json.hpp"
#include <cereal/types/polymorphic.hpp>

#include <memory>

struct Handle;

struct Node : public std::enable_shared_from_this<Node> {
    Node(int val):data(val){}
    virtual ~Node(){}
    std::map<std::string, std::shared_ptr<Node>> children;
    std::shared_ptr<Handle> handle;

    template <class Archive>
    void serialize(Archive & archive){
        archive(CEREAL_NVP(handle), CEREAL_NVP(children), CEREAL_NVP(data));
    }

    std::shared_ptr<Node> shared(){
        return shared_from_this();
    }

    template <class Archive>
    static void load_and_allocate(Archive & archive, cereal::allocate<Node> &allocate){
        allocate(-1);
        archive(cereal::make_nvp("data", allocate->data));
    }

    int data = 0;
};

struct DerivedNode : public Node {
    DerivedNode(int val):Node(val){}

    template <class Archive>
    static void load_and_allocate(Archive & archive, cereal::allocate<DerivedNode> &allocate){
        allocate(-1);
        archive(cereal::make_nvp("data", allocate->data));
    }
};

CEREAL_REGISTER_TYPE(Node);
CEREAL_REGISTER_TYPE(DerivedNode);

struct Definition;
struct Handle : public std::enable_shared_from_this<Handle> {
    Handle(int id):id(id){}
    int id = 0;
    std::shared_ptr<Definition> definition;

    template <class Archive>
    void serialize(Archive & archive){
        int testInt = 0;
        int *pointInt = &testInt;
        archive.extract(cereal::make_nvp("testInt", testInt), cereal::make_nvp("pointInt", pointInt));
        std::cout << testInt << ": " << *pointInt << std::endl;
        archive(CEREAL_NVP(id), CEREAL_NVP(definition));
    }

    template <class Archive>
    static void load_and_allocate(Archive & archive, cereal::allocate<Handle> &allocate){
        int theId;
        archive(cereal::make_nvp("id", theId));
        allocate(theId);
    }
};

struct Definition : public std::enable_shared_from_this<Definition> {
    virtual ~Definition(){}
    Definition(int id):id(id){}
    int id = 0;
    std::vector<std::weak_ptr<Handle>> handles;

    template <class Archive>
    void serialize(Archive & archive){
        archive(CEREAL_NVP(id), CEREAL_NVP(handles));
    }

    template <class Archive>
    static void load_and_allocate(Archive & archive, cereal::allocate<Definition> &allocate){
        int theId;
        archive(cereal::make_nvp("id", theId));
        allocate(theId);
    }
};

struct DerivedDefinition : public Definition {
    DerivedDefinition(int id):Definition(id){}

    template <class Archive>
    static void load_and_allocate(Archive & archive, cereal::allocate<DerivedDefinition> &allocate){
        int theId;
        archive(cereal::make_nvp("id", theId));
        allocate(theId);
    }
};

CEREAL_REGISTER_TYPE(Definition);
CEREAL_REGISTER_TYPE(DerivedDefinition);

void saveTest(){
    int testInt = 10;
    int *pointInt = &testInt;
    std::stringstream stream;
    {
        std::shared_ptr<Node> arm = std::make_shared<DerivedNode>(1);
        std::shared_ptr<Node> rock1 = std::make_shared<DerivedNode>(2);
        std::shared_ptr<Node> rock2 = std::make_shared<DerivedNode>(3);
        arm->children["rock1"] = rock1;
        arm->children["rock2"] = rock2;

        std::shared_ptr<Definition> definition = std::make_shared<DerivedDefinition>(1);
        rock1->handle = std::make_shared<Handle>(1);
        rock2->handle = std::make_shared<Handle>(2);
        rock1->handle->definition = definition;
        rock2->handle->definition = definition;

        cereal::JSONOutputArchive archive(stream);
        archive(cereal::make_nvp("arm", arm));
    }
    std::cout << stream.str() << std::endl;
    std::cout << std::endl;
    {
        cereal::JSONInputArchive archive(stream);
        archive.add(cereal::make_nvp("testInt", testInt), cereal::make_nvp("pointInt", pointInt));
        testInt = 30;

        std::shared_ptr<DerivedNode> arm;
        archive(cereal::make_nvp("arm", arm));

        arm->shared_from_this();
        std::cout << arm->shared()->data << std::endl;

        cereal::JSONOutputArchive outArchive(stream);
        outArchive(cereal::make_nvp("arm", arm));

        std::cout << stream.str() << std::endl;
        std::cout << std::endl;

        archive(cereal::make_nvp("arm", arm));
        std::cout << "Arm Child 0 Definition ID: " << arm->children["rock1"]->handle->definition->id << std::endl;
        std::cout << "Arm Child 1 Definition ID: " << arm->children["rock2"]->handle->definition->id << std::endl;
        arm->children["rock1"]->handle->shared_from_this();
        arm->children["rock1"]->handle->definition->shared_from_this();
    }

    std::cout << "Done" << std::endl;
}

int main(){
    saveTest();
}

[AzothAmmo]

Is it possible to make a version of this without the extract stuff? Also I'm not incredibly familiar with the mechanics of enable_shared_from_this, would you mind explaining which weak_ptr you refer to? Your code only seems to have a vector of them which get saved but never loaded.

[Devacor]

Here's a way better example, apologies, the previous example was just me sketching on an existing test I was working on (and also missed saving handles, but that wasn't the issue):

#include <strstream>

#include "cereal/cereal.hpp"
#include "cereal/types/map.hpp"
#include "cereal/types/vector.hpp"
#include "cereal/types/memory.hpp"
#include "cereal/types/string.hpp"
#include "cereal/types/base_class.hpp"

#include "cereal/archives/json.hpp"
#include <cereal/types/polymorphic.hpp>

#include <memory>

struct Handle;

struct Node : public std::enable_shared_from_this<Node> {
    Node(int val):data(val){}
    virtual ~Node(){}

    template <class Archive>
    void serialize(Archive & archive){
        archive(CEREAL_NVP(data));
    }

    template <class Archive>
    static void load_and_allocate(Archive & archive, cereal::allocate<Node> &allocate){
        int data;
        archive(cereal::make_nvp("data", data));
        allocate(data);
    }

    int data = 0;
};

void saveTest(){
    int testInt = 10;
    int *pointInt = &testInt;
    std::stringstream stream;
    {
        std::shared_ptr<Node> node = std::make_shared<Node>(1);

        cereal::JSONOutputArchive archive(stream);
        archive(cereal::make_nvp("node", node));
    }
    std::cout << stream.str() << std::endl;
    std::cout << std::endl;
    {
        std::shared_ptr<Node> node;

        cereal::JSONInputArchive archive(stream);
        archive(cereal::make_nvp("node", node));

        node->shared_from_this(); //CRASH HERE
        std::cout << "We good?" << std::endl;
    }

    std::cout << "Done" << std::endl;
}

int main(){
    saveTest();
}

enable_shared_from_this is a class you can derive your own classes from. When you do, enable_shared_from_this has an internal weak_ptr reference to "this", and you can access shared_from_this() which returns a locked copy of that weak_ptr.

The issue is that when you perform your allocation within load_and_allocate, the weak_ptr ref never gets any love, and so it sits stagnant and if you try to call shared_from_this() you get a crash when it tries to lock the janky weak_ptr ref it should have stored.

[Devacor]

http://gcc.gnu.org/onlinedocs/gcc-4.6.4/libstdc++/api/a01043_source.html search for class enable_shared_from_this for an example implementation.

[AzothAmmo]

You are right - the way this works is that the constructor for std::shared_ptr is actually what ends up doing the assignment of the weak_ptr contained within enable_shared_from_this.

For load_and_allocate, we initialize the shared_ptr with aligned_storage of the appropriate size, which is then cast to the true type being loaded. So I'm guessing that shared_ptr is still trying to assign something to this weak_ptr, even though it's just junk. When we give this back to the user using cereal::allocate, the placement new comes along and wipes this out anyway, default initializing the weak_ptr, ultimately giving the error you described.

The only way for that weak_ptr to be properly initialized is through the constructor of shared_ptr. Might be tricky to fix this without getting too hacky.

[Devacor]

nods I was thinking the same. I rely on enable_shared_from_this and I would totally appreciate effort in fixing it. But I don't think it'll be easy to do without somehow breaking encapsulation in a hacky way. I just hope it can be done without resorting to std implementation specific work-arounds.

[AzothAmmo]

OK I have an extremely hacky solution for this working that should (I think?) work independently from implementation. The gist is this:

1. Allocate our aligned_storage just as we are (henceforth referred to as (1))
2. Allocate a second aligned_storage for std::enable_shared_from_this<T> (referred to as (2))
3. memcpy the contents of (1) static_cast to enable_shared_from_this<T> * into (2)
4. Let the user do their allocation
5. memcpy the contents from (2) back into (1)

Thoughts on this monstrosity?

[Devacor]

Unfortunately I do not have very much experience using aligned_storage or placement new. The steps you outline seem to be reasonable, however. I don't know of a better way certainly.

As long as the weak_ptr is pointing to the final object (the same one as the owning shared_ptr) that seems fine.

[Devacor]

Oh no! We have a problem, capn'

Polymorphic stuff is busted. This was one of the things I was a bit worried about, but didn't know if it would be an issue or not.

#include <strstream>

#include "cereal/cereal.hpp"
#include "cereal/types/map.hpp"
#include "cereal/types/vector.hpp"
#include "cereal/types/memory.hpp"
#include "cereal/types/string.hpp"
#include "cereal/types/base_class.hpp"

#include "cereal/archives/json.hpp"
#include <cereal/types/polymorphic.hpp>

#include <memory>

struct Handle;

struct Node : public std::enable_shared_from_this<Node> {
    Node(int val):data(val){}
    virtual ~Node(){}

    template <class Archive>
    void serialize(Archive & archive){
        archive(CEREAL_NVP(data));
    }

    template <class Archive>
    static void load_and_allocate(Archive & archive, cereal::allocate<Node> &allocate){
        int data;
        archive(cereal::make_nvp("data", data));
        allocate(data);
    }

    int data = 0;
};

struct DerivedNode : public Node {
    DerivedNode(int val):Node(val){}

    template <class Archive>
    void serialize(Archive & archive){
        archive(cereal::make_nvp("base", cereal::base_class<Node>(this)));
    }

    template <class Archive>
    static void load_and_allocate(Archive & archive, cereal::allocate<DerivedNode> &allocate){
        allocate(-1);
        archive(cereal::make_nvp("base", cereal::base_class<Node>(allocate.ptr())));
    }
};

CEREAL_REGISTER_TYPE(Node);
CEREAL_REGISTER_TYPE(DerivedNode);

void saveTest(){
    int testInt = 10;
    int *pointInt = &testInt;
    std::stringstream stream;
    {
        std::shared_ptr<Node> node = std::make_shared<DerivedNode>(1);

        cereal::JSONOutputArchive archive(stream);
        archive(cereal::make_nvp("node", node));
    }
    std::cout << stream.str() << std::endl;
    std::cout << std::endl;
    {
        std::shared_ptr<DerivedNode> node;

        cereal::JSONInputArchive archive(stream);
        archive(cereal::make_nvp("node", node));

        node->shared_from_this();
        std::cout << "We good?" << std::endl;
    }

    std::cout << "Done" << std::endl;
}

int main(){
    saveTest();
}

[AzothAmmo]

You are right, there is still an issue with polymorphic types. cereal loads the true type for your polymorphic type (let's say the class hierarchy is enable_shared_from_this<A> -> A -> B), which may be B in this example, and then runs into the problem that B is not derived from enable_shared_from_this<B>, but enable_shared_from_this<A>.

This can be fixed but there will be some limitations on the hierarchies supported by cereal. For a polymorphic type, stored in a shared_ptr, deriving from enable_shared_from_this, and finally using load_and_allocate: either the current pointer type or its true pointer type (polymorphism) must be the class that derived from enable_shared_from_this. In addition the way I'm going to implement this it will attempt to cast to the most base class, so having both the parent and derived class inherit from enable_share_from_this will not be supported.

[AzothAmmo]

Also unrelated to this issue, there is no need to register the base class of a polymorphic hierarchy (e.g. Node in your example).

[AzothAmmo]

Might not be fixed as soon as I thought - it's pretty tricky to do. Unfortunately the standard doesn't define an implementation for the internals of enable_shared_from_this, so I hesitate to implement a solution that relies on any one implementation (although clang, gcc, and msvc all use a weak_ptr internally).

If you want to locally fix this for yourself in the meantime, you can do something like this:

template <class T>
struct Dummy
{
  std::weak_ptr<T> weak;
};

int main()
{
  // loading up some stuff
  std::shared_ptr<MyClass> bla;
  ar( bla );
  reinterpret_cast<Dummy<MyClass> *>( static_cast<std::enable_shared_from_this<MyClass> *>( bla.get() ) )->weak = bla;
}

This should work over any of the compilers I mentioned and properly set up the internal weak_ptr.

[Devacor]

Thanks dude. Awesome response!

[AzothAmmo]

Should be completely good to go, and done in a non hacky way! Test this on VS for us, we haven't had the chance yet.