Search code examples
c++c++11pointerssmart-pointersweak-ptr

How should I store a list of weak pointers in the factory class?


There is a Factory class in my project, which returns std::shared_ptr<MyClass>, if the Create method is called. The factory is not the owner, but it maintains a list of created objects. Sometimes the factory needs to iterate over the created objects which are still used, or count them.

I implemented this using std::vector<std::weak_ptr<MyClass>>. The iteration looks like this:

for (std::weak_ptr<MyClass> wp : weak_list) {
    if (auto sp = wp.lock()) {
        //..
    }
}

The Create function:

std::shared_ptr<MyClass> Create(/* ... */) {
    std::shared_ptr<MyClass> obj = std::make_shared<MyClass>();

    //...

    weak_list.push_back(obj);
    return obj;
}

Is vector the best container for a list like this? Maybe std::set would be better.

If I implement this using a vector, I have to check regularly for expired pointers in the list and remove them

What is the preferred way to store a list of weak pointers in the factory class?


Solution

  • By default use vector.

    Sets will not self clean expired pointers, and the manual cleaning process is the same.

    Consider cleaning dead elements before iterating, and iterating over a copy of the weak list (in case the iteration mutates the weak list).

    If you need immediate cleaning of the dangling weak ptrs, modify the shared ptr to unregister itself from the weak list when the last strong reference goes away. This requires some work to do with make shared involving aligned storage, manual construction and destruction, a helper struct and the aliasing contructor to shared ptr.

    template<class T>
    struct shared_helper {
      typename std::aligned_storage<sizeof(T),alignof(T)::type data;
      T* get(){ return reinterpret_cast<T*>(&data); }
      bool constructed = false;
      weak_list<T>* list=0;
      std::weak_ptr<T> self;
      ~shared_helper(){
        if (constructed){
          get()->~T();
          constructed=false;
        }
        if (list) list->erase(self);
      }
      template<class...Ts>
      T* emplace(Ts&&...ts){
        T* r = ::new( (void*)&data ) T(std::forward<Ts>(ts)...);
        constructed = true;
        return r;
      }
    };
    template<class T, class...Args>
    std::shared_ptr<T> make_and_register( weak_list<T>& list, Args&&...args ){
      auto r = std::make_shared<shared_helper<T>>();
      if(!r) return {};
      r->emplace( std::forward<Args>(args)... );
      std::shared_ptr<T> ptr( r, r->get() );
      r->self = ptr;
      list.insert(ptr);
      r->list = &list
      return ptr;
    }
    

    which is a bit obtuse, untested and uncompiled. But in this case, a set or unordered set for your weak list makes sense, as we use fast lookup to instantly clean the weak list when the last strong reference expires.