Search code examples
c++c++11boostshared-ptrsmart-pointers

std::enable_shared_from_this with different owner


And yet another enable_shared_from_this question: Basically, I got three things.

  1. System classes that contain application logic and might or might not be event listeners.
  2. Some kind of EventManager, that maps events to shared_ptrs of interested systems.
  3. Some kind of SystemManager, that holds a std::list of shared_ptrs to System objects.

Now, each system should be able to register itself with the EventManager for all kinds of events it is interested in. Moreover, they should be able to de-register themselves for these events if they aren't interested in them any more.

However, I'm not sure how to create systems in the system manager and sign them up in a way that avoids two ownership groups.

From the SystemManager:

void SystemManager::AddSystem(GameSystem* system)
{
    // Take ownership of the specified system.
    systems.push_back(std::shared_ptr<GameSystem>(system));
}

From the EventManager:

typedef std::shared_ptr<IEventListener> EventListenerPtr;

void EventManager::AddListener(EventListenerPtr const & listener, HashedString const & eventType)
{
    // Get the event type entry in the listeners map.
    std::map<unsigned long, std::list<EventListenerPtr>>::iterator iterator = this->listeners.find(eventType.getHash());

    if (iterator != this->listeners.end())
    {
        std::list<EventListenerPtr> & eventListeners = iterator->second;

        // Add listener.
        eventListeners.push_back(listener);
    }
    else
    {
        // Add new entry to listeners map, removed for sake of simplicity of this example.
    }
}

From some arbitrary system that wants to receive events:

void RenderSystem::InitSystem(std::shared_ptr<GameInfrastructure> game)
{
    // BAD: Creates second ownership group.
    this->game->eventManager->AddListener(std::shared_ptr<IEventListener>(this), AppWindowChangedEvent::AppWindowChangedEventType);
}

I have read my references recommending enable_shared_from_this. However, as the systems don't own themselves, they don't have access to the shared_ptr held by the SystemManager who originally created the systems and took ownership of them.

Any ideas of an elegant way to solve this?


Solution

  • Your first problem is IEventListener. I'm guessing it is an interface to a simple callback.

    Don't use inheritance for interfaces to simple callbacks, use type erasure on a simple callback.

    typedef std::function<void(EventData)> EventListener;
    typedef std::weak_ptr<EventListener> wpEventListener;
    typedef std::shared_ptr<EventListener> EventToken;
    
    EventToken EventManager::AddListener(EventListener listener, HashedString const & eventType)
    {
      // note type changes (3 of them!) -- change the type of this->listeners to match
      // probably HashedString should have a std::hash<HashedString> specialization to make this
      // less painful
      // also should be auto iterator = -- the type of an iterator is not interesting.
      std::unordered_map<unsigned long, std::vector<wpEventListener>>::iterator iterator = this->listeners.find(eventType.getHash());
    
      if (iterator != this->listeners.end())
      {
        auto & eventListeners = iterator->second;
    
        EventToken retval = std::make_shared<EventListener>(std::move(listener));
        eventListeners.push_back(retval);
        return retval;
      } else {
        // Add new entry to listeners map, removed for sake of simplicity of this example.
      }
    }
    

    now when you install Listener, you are responsible to hold your EventToken until you no longer want to listen to it. When you no longer want to listen to it, EventToken.reset().

    In the EventManager, when you iterate over people listening, do auto listener = it->lock(); if (!listener) continue;, and in addition do a quick erase-remove-if to remove dead weak_ptrs.

    Now our EventManager doesn't own the lifetime of its listeners, they can go away (almost) whenever they want. The EventManager notices the next time an event is invoked, and cleans up the dead ones.

    You can retrofit this into using IEventListener, but even here you'll want a weak pointer, not a shared pointer. Using shared pointers everywhere is going to get you in trouble, as you'll have circular self-supporting references and resource leaks aplenty.

    Finally, shared_from_this actually lets you access the shared_ptr that owns you. It means that when a shared_ptr is constructed, a weak_ptr is stored in the shared_from_this, which can later be extracted even if you only have the this pointer.

    I typically find this a bad sign: you should rarely be upgrading raw pointers to shared pointers in random contexts. The lifetime semantics of a method should be obvious from its interface, and taking a T* and turning it internally into a shared_ptr<T> makes the lifetime semantics completely unclear.