Visitor Pattern and std::shared_ptr

Question

I am using the Visitor design pattern in my application for message processing. For example:

class AbstractMessageVisitor;

class AbstractMessage {
public:
      virtual void accept(AbstractMessageVisitor& visitor) const = 0;
};

class A : public AbstractMessage {
public:
    void accept(AbstractMessageVisitor& visitor) const override { visitor.visit(*this); }
};
class B : public AbstractMessage { 
    //... 
};

class AbstractMessageVisitor {
    friend class A;
    friend class B;

protected:
    virtual void visit(const A&) {};
    virtual void visit(const B&) {};
};

In my application, instances of the messages are being created as std::shared_ptr. If I "receive" one of these messages created by the factory, I would "visit" it by doing the following:

class MessageHandler : public AbstractMessageVisitor {
public:
    void handleMessage(std::shared_ptr<const AbstractMessage> msg) {
         msg->accept(*this);
    }

protected:
    void visit(const A& msg);
    void visit(const B& msg);
};

In this case, I realized that my visit methods may need to "save the message for later." Since I know that the message that is being visited is managed by a std::shared_ptr, I figured I could just copy the shared_ptr to use later.

However, this is where I encountered my problem; within the visit methods there is no shared_ptr.

Here are some of the solutions I found to the problem:

Option 1

Add a member variable to MessageHandler class that temporarily stores the shared_ptr while the message is being visited. If the visit method needs the pointer, it makes a copy of this member variable.

class MessageHandler : public AbstractMessageVisitor {
public:
    void handleMessage(std::shared_ptr<const AbstractMessage> msg) {
         _ptr = msg;
         msg->accept(*this);
         _ptr.reset();
    }

protected:
    void visit(const A& msg) {
        auto thePointer = std::static_pointer_cast<const A>(_ptr);
    }
    void visit(const B& msg);

private:
    std::shared_ptr<AbstractMessage> _ptr;
};

Obviously, this has many problems with it. You have to cast the temporary shared pointer into the appropriate type. This kind of defeats the purpose of the visitor pattern. You have this "shared state" that must be kept as a member variable.

Option 2

Inherit from std::enable_shared_from_this.

class A : public AbstractMessage, std::enable_shared_from_this<A> {
public:
    inline auto ptr() const { return shared_from_this(); }
    //...
};

However, this only works if you can be guaranteed that the class is owned by a shared_ptr. On GCC, if it is not, it looks like an exception will be thrown or, if exceptions are disabled, the program will immediately exit.

Looking at the implementation of enable_shared_from_this, I wonder why shared_from_this could not just return a nullptr if the object is NOT owned by a shared_ptr, but alas...

Option 3

Just make the object "cloneable".

class A : public AbstractMessage {
public:
    std::shared_ptr cloneShared() { return std::make_shared<A>(*this); }
    //...
};

This isn't really a solution, as it is not taking advantage of the fact that a shared_ptr to the object already exists.

---

So, my questions are:

• Is there a way to accomplish what I am trying to do?
• Perhaps this problem is coming from a flaw in my design; is there something I should change?

Answer 1

If the instances of your class are supposed to have a valid associted shared_ptr, they are supposed to be on dynamic storage and either adopted or created by shared_ptr. Objects with automatic storage(class instance members, function local objects) and static storage objects(static/extern objects) can't be shared. As a general rule, when a class inherits from CRTP base enable_shared_from_this, it must hide its constructors from public and introduce some create or whatever method that returns shared_ptrs instead. The method can simply be a forwarder to std::make_shared:

class myclass:
    public std::enabled_shared_from_this<myclass>
{
private:
    myclass();
    myclass(myclass const&);
    myclass(myclass &&);
    //Make all other constructors private
public:
    template<typename ...args>
    static auto create(args&& ... params){
        //return std::make_shared<myclass>(std::forward<args>(params)...);
        return new myclass{ std::forward<args>(params)... };
    };
    //...rest of the class
};

Nothing is a must, but this was just the general pattern for using enable_shared_from_this. This way you guarantee that all instances are in fact shared and safe to use shared_from_this.

Edit:=============================

This is an exception to general advice to use make_shared instead of new. With all constructors made private, you'll need new.

Answer 2

I believe I found an answer. It is based upon the following two references:

https://en.cppreference.com/w/cpp/memory/enable_shared_from_this/weak_from_this

Check whether an object derived from enable_shared_from_this is managed by shared_ptr?

It appears that, if you use the weak_from_this() method instead of shared_from_this(), an exception will NOT be generated. Instead the behavior is as I would expect, if the object is NOT owned by a shared_ptr, the weak_ptr returned will be invalid (i.e., .lock() will return a nullptr).

Thus, I can do the following in my example:

class A : public AbstractMessage, std::enable_shared_from_this<A> {
public:
    inline auto ptr() const noexcept { return weak_from_this().lock(); }
    void accept(AbstractMessageVisitor& visitor) const override { visitor.visit(*this); }
};

class MessageHandler : public AbstractMessageVisitor {
public:
    void handleMessage(std::shared_ptr<const AbstractMessage> msg) {
         msg->accept(*this);
    }

protected:
    void visit(const A& msg) {
        _savedForLater = msg.ptr();
    }
    void visit(const B& msg);

private:
    std::shared_ptr<A> _savedForLater;
};

Answer 3

THis

class AbstractMessageVisitor {
    friend class A;
    friend class B;

protected:
    virtual void visit(const A&) {};
    virtual void visit(const B&) {};
};

Is very strange. Your Visitor has to be told expressly about each message type.

Surely it should be

class AbstractMessageVisitor {


protected:
    virtual void visit(const AbstractMessage&) {};
};

Ie the visitor deals with instances of Abstract message derived classes. I would have

using AbstractMessagePtr = std::shared_ptr<AbstractMessage>;

...
   virtual void visit(AbstractMessagePtr msg) {};

ie always be dealing with shared_ptr to messages

If you need to keep the pointer for a while just have a member vaiable of type AbstractMessagePtr in the class