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

Question

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?

the answer will always be - it depends. You should try both, measure and pick the better one. Do not use `std::make_shared` if you want to use `std::weak_ptr` - https://stackoverflow.com/questions/32113594/weak-ptr-make-shared-and-memory-deallocation — luantkow, Aug 20 '17 at 10:37
`weak_list` is `std::vector>`. The `obj` `shared_ptr` is returned, that is why I called `make_shared` — Iter Ator, Aug 20 '17 at 10:42
I understand that you return the `shared_ptr` but you keep `weak_ptr` for the new object. If you create `std::shared_ptr` with `std::make_shared` the allocated memory will be released only after the last `std::weak_ptr` is destroyed. It is not memory leak, but your application may use more memory than it could. — luantkow, Aug 20 '17 at 10:52

score 4 · Accepted Answer · answered Aug 20 '17 at 14:10

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.

"iterating over a copy of the weak list (in case the iteration mutates the weak list)"—yep, have experienced weird behavior by not doing this myself. — HelloGoodbye, Mar 14 '19 at 09:37

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

1 Answers1