Search code examples
c++serializationboostshared-ptr

bad weak pointer after deserializing object that has a shared pointer to base

I'm getting bad weak pointers after serializing then deserializing objects that have-a shared pointer to virtual base class, using Boost.Serialization.

My minimal example demonstrates:

  • Abstract base class Base, which has a shared pointer to Base with default value nullptr.
  • Concrete derived class SharedPtrHolder. This type overrides virtual method check_shared_from_this which uses shared_from_this. This call fails due to bad weak pointer after de-serialization of another type HasASharedPtrHolder
  • Type HasASharedPtrHolder, owning a shared pointer to Base.

The problem: after de-serialization, calls to shared_from_this fail due to bad weak pointers. I'm baffled -- after deserialization HasASharedPtrHolder in fact has shared pointers, they don't fail to deserialize. But I can't call shared_from_this on them!!!

My real use case uses visitor pattern, and the bad weak pointers occur in the Visit calls. Before serialization, fine. After, nope. I think this MWE demonstrates the problem.

I would love help with this. Thanks!

MWE:

#include <memory>

#include <boost/archive/text_oarchive.hpp>
#include <boost/archive/text_iarchive.hpp>
#include <boost/serialization/export.hpp>
#include <boost/serialization/shared_ptr.hpp>
#include <boost/serialization/base_object.hpp>
#include <boost/serialization/nvp.hpp>

//this #define MUST appear before #include <boost/test/unit_test.hpp>
#define BOOST_TEST_MODULE "MWE"
#define BOOST_TEST_DYN_LINK

#include <boost/test/unit_test.hpp>
#include <fstream>

// abstract, and owns a pointer to Base.
class Base{
public:
    Base() = default;
    virtual ~Base() = default;


    void set_ptr(std::shared_ptr<Base> const& n){
        this->_next = n;
    }

    const std::shared_ptr<Base> get_ptr() const{
        return this->_next;
    }

    virtual void do_thing_using_shared_from_this(){
        throw std::runtime_error("failed to override, this is the base");
    }

    virtual void print(){
        std::cout << "this is base, this should have been overridden" << std::endl;
    }

private:

    std::shared_ptr<Base> _next = nullptr;


    friend class boost::serialization::access;

    template <typename Archive>
    void serialize(Archive& ar, const unsigned version) {
        ar & _next;
    }



};

// concrete, with a method using `shared_from_this`
// this is a proxy for a visitable type.
class SharedPtrHolder: public virtual Base, public std::enable_shared_from_this<SharedPtrHolder>
{
public:
    SharedPtrHolder() = default;
    virtual ~SharedPtrHolder() = default;
        
    void check_shared_from_this() const{
        std::shared_ptr<const SharedPtrHolder> another_shared_ptr = this->shared_from_this();
    }

    virtual void do_thing_using_shared_from_this() override
    {
        auto as_shared = this->shared_from_this();
    }

    virtual void print() override{
        std::cout << "this is derived" << std::endl;
    }
private:




    friend class boost::serialization::access;

    template <typename Archive>
    void serialize(Archive& ar, const unsigned version) {
        ar & BOOST_SERIALIZATION_BASE_OBJECT_NVP(Base);
    }

};



// this is a proxy for a type that owns visitable pointers.
class HasASharedPtrHolder{

public:
    HasASharedPtrHolder(std::shared_ptr<Base> const& p): _ptr(p) 
    {}

    HasASharedPtrHolder() = default;

    ~HasASharedPtrHolder() = default;

    void set_ptr(std::shared_ptr<Base> const& n){
        this->_ptr = n;
    }

    void check_shared_from_this() const{
        _ptr->do_thing_using_shared_from_this(); // this call fails after de-serialization, due to bad weak pointers
    }

    void call_a_virtual_function(){
        _ptr->print();
    }
private:
    std::shared_ptr<Base> _ptr = nullptr;


    friend class boost::serialization::access;

    template <typename Archive>
    void serialize(Archive& ar, const unsigned version) {
        ar.template register_type<SharedPtrHolder>(); // have to register, because have pointer to base
        ar & _ptr; // serialize the data this type owns
    }


};


BOOST_AUTO_TEST_SUITE(boost_serialization)



BOOST_AUTO_TEST_CASE(serialize_deserialize_has_a)
{
    { // to create a scope


        auto a_ptr = std::make_shared<SharedPtrHolder>();
        auto b_ptr = std::make_shared<SharedPtrHolder>();

        b_ptr->set_ptr(a_ptr);

        HasASharedPtrHolder sys(b_ptr);

        std::ofstream fout("serialization_basic");
        
        boost::archive::text_oarchive oa(fout);
        
        sys.check_shared_from_this();
        // write class instance to archive
        oa << sys;
    }
    
    
    {
        std::ifstream fin("serialization_basic");
        
        boost::archive::text_iarchive ia(fin);
        // read class state from archive

        HasASharedPtrHolder sys;
 
        ia >> sys;

        sys.call_a_virtual_function(); // this call is fine
        sys.check_shared_from_this(); // bad weak pointer, due to shared_from_this
    }

}

BOOST_AUTO_TEST_SUITE_END()
Solution

  • You're serializing through shared_ptr<Base>. Base does not publicly inheriting from enable_shared_from_this. That's your problem.

    If you have nodes that require it, you need to serialize them as such.

    A slight reshuffle seems to make sense. Given the invariants a static_pointer_cast should be enough to get shared_ptr<Node const> from the shared_from_this() pointer:

    Live On Coliru

    #include <boost/archive/text_iarchive.hpp>
#include <boost/archive/text_oarchive.hpp>
#include <boost/serialization/base_object.hpp>
#include <boost/serialization/export.hpp>
#include <boost/serialization/shared_ptr.hpp>

// this #define MUST appear before #include <boost/test/unit_test.hpp>
#define BOOST_TEST_MODULE "MWE"
// #define BOOST_TEST_DYN_LINK

#include <boost/test/unit_test.hpp>
#include <fstream>

// abstract, and owns a pointer to Base.
using SBase = std::shared_ptr<class Base>;

class Base : public std::enable_shared_from_this<Base> {
  public:
    Base()          = default;
    virtual ~Base() = default;

    void next(SBase const& n)   { _next = n;                                   } 
    const SBase  next() const   { return _next;                                } 
    virtual void check_shared() { throw std::runtime_error("not implemented"); } 
    virtual void print()        { std::cout << "(abstract)" << std::endl;      } 

  private:
    SBase _next = nullptr;

    friend class boost::serialization::access;
    template <typename Ar> void serialize(Ar& ar, unsigned) { ar& _next; }
};

// concrete, with a method using `shared_from_this`
// this is a proxy for a visitable type.
class Node : public /*virtual*/ Base {
  public:
    Node()          = default;
    virtual ~Node() = default;

    auto check_shared_from_this() const {
        // return std::dynamic_pointer_cast<Node const>(shared_from_this());
        return std::static_pointer_cast<Node const>(shared_from_this());
    }

    virtual void check_shared() override { auto as_shared = shared_from_this(); }
    virtual void print() override { std::cout << "this is Node" << std::endl; }

  private:
    friend class boost::serialization::access;
    template <typename Ar> void serialize(Ar& ar, unsigned) {
        ar& boost::serialization::base_object<Base>(*this);
    }
};

BOOST_CLASS_EXPORT(Base)
BOOST_CLASS_EXPORT(Node)

// this is a proxy for a type that owns visitable pointers.
class System {
  public:
    System(SBase const& p = {}) : _ptr(p) {}

    void set(SBase const& n) { _ptr = n;             } 
    void exercise() const    { _ptr->check_shared(); } 
    void print()             { _ptr->print();        } 

  private:
    SBase _ptr = nullptr;

    friend class boost::serialization::access;
    template <typename Ar> void serialize(Ar& ar, unsigned) { ar& _ptr; }
};

BOOST_AUTO_TEST_SUITE(boost_serialization)

BOOST_AUTO_TEST_CASE(repro) {
    std::stringstream ss;
    {
        SBase a = std::make_shared<Node>(), b = std::make_shared<Node>();
        b->next(a);

        System sys(b);
        sys.exercise();

        boost::archive::text_oarchive oa(ss);
        oa << sys;
    }

    {
        System sys;
        {
            boost::archive::text_iarchive ia(ss);
            ia >> sys;
        }

        sys.print(); // this call is fine
        sys.exercise();
    }
}

BOOST_AUTO_TEST_SUITE_END()

    Output

    enter image description here

    Side notes:

    • Prefer registering types outside of serialization methods
    • Use NVP wrappers consistently or don't if you don't need them anyways
    • avoid virtual inheritance if possible: they invite numerous extra subtle sources of undefined behaviour and have a runtime/storage/code size cost

    I've taken the liberty to rename things during review - just to aid my understanding of the code's intent.