C ++ weak_ptr.lock () segfault

Question

So, I have this function to add listeners, and it converts the general class pointers, so I can call later when I get a notification.

void registerListener(std::shared_ptr<T> listener) { if (!listener) { qCWarning(OBSERVER_LOGGER) << "Attempted to register a null observer."; return; } // TODO make a foreach function that removes dead listeners to get rid of this code dupe for (auto iter=listeners.begin(); iter != listeners.end(); ) { if (auto shared = iter->lock()) { if (listener == shared) { return; } iter++; } else { iter = listeners.erase(iter); } } auto weak = std::weak_ptr<T>(listener); listeners.push_back(weak); } void notify(std::function<void(std::shared_ptr<T>)> onNotify) { // TODO make a foreach function that removes dead listeners to get rid of this code dupe for (auto iter=listeners.begin(); iter != listeners.end(); ) { if (auto shared = iter->lock()) { onNotify(shared); iter++; } else { iter = listeners.erase(iter); } } } private: std::vector<std::weak_ptr<T>> listeners; 

and for some reason "iter-> lock ()" segfaults. I will say that this is a Qt application, but I intentionally did not create ANY threads (that I know), so I am just confused by what I am doing wrong so that these weak fragments break. Therefore, if I run it in gdb, it works fine. But if I set "set disable-randomization off", then I get an error. Therefore, I feel that this is some kind of strange problem when with uninitialized variables. If this helps, this is the stack when I crashed into gdb.

 #0 0x00007f856bd8beec in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_get_use_count() const () #1 0x00007f856bd844a8 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_add_ref_lock_nothrow() () #2 0x00007f856bd9cd7d in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count(std::__weak_count<(__gnu_cxx::_Lock_policy)2> const&, std::nothrow_t) () #3 0x00007f856bda9948 in std::__shared_ptr<IEntityListener<Assignment>, (__gnu_cxx::_Lock_policy)2>::__shared_ptr(std::__weak_ptr<IEntityListener<Assignment>, (__gnu_cxx::_Lock_policy)2> const&, std::nothrow_t) () #4 0x00007f856bda8a62 in std::shared_ptr<IEntityListener<Assignment> >::shared_ptr(std::weak_ptr<IEntityListener<Assignment> > const&, std::nothrow_t) () #5 0x00007f856bda701a in std::weak_ptr<IEntityListener<Assignment> >::lock() const () #6 0x00007f856bda5624 in Observer<IEntityListener<Assignment> >::notify(std::function<void (std::shared_ptr<IEntityListener<Assignment> >)>) () #7 0x00007f856bda3a1a in EntityObserver<Assignment>::notifyCreated(std::shared_ptr<Assignment>) () 

EDIT: Michael Burr posted an opportunity to listen to listeners, while new listeners added what could happen completely. This will cause the iterator to be invalid, and when I go to the weak_ptr.lock () call in the memory section, which is not weak_ptr, BOOM. I think that there is morality, somewhere I just have to find it.

Answer 1

When notify() is called, is it possible that a function called through the onNotify() function object will cause registerListener() be called indirectly (or some other code that can add or remove entries in the listeners collection) ,?

If so, then the iter used in the notify() for loop may not be valid. You might want to change notify() to look something like this: a queue for all shared_ptr objects to be notified, so it doesn't matter if the listeners collection changes during any of the onNotify() :

 #include <queue> void notify(std::function<void(std::shared_ptr<T>)> onNotify) { std::queue<std::shared_ptr<T>> notify_targets; for (auto iter=listeners.begin(); iter != listeners.end(); ) { if (auto shared = iter->lock()) { notify_targets.push(shared); iter++; } else { iter = listeners.erase(iter); } } while (!notify_targets.empty()) { onNotify(notify_targets.front()); notify_targets.pop(); } } 

Answer 2

I cannot find the obvious problem in the code you are showing. Therefore, I assume that the problem is in the code that you are not showing. Keep in mind that objects decorated with Q_OBJECT also have qt related lifecycle management. There may be some interference ...

Maybe this will help if you compare your code with the code I am posting here and you quickly find the key difference.

 #include <vector> #include <algorithm> #include <memory> template <typename _Observable> class IObserver { public: virtual ~IObserver() {} virtual void OnChange(const _Observable* what) = 0; }; template <class T> class CObservable : public IObserver<T> // Make sure this class breaks once IObserver<> changes. { public: typedef IObserver<T> Observer_t; typedef std::weak_ptr<Observer_t> Observer_reference; typedef std::shared_ptr<Observer_t> Observer_strong_reference; typedef T Class_t; typedef std::vector<Observer_reference> ObserverRefCollection; private: ObserverRefCollection m_observers; void CleanupZombies() { m_observers.erase(std::remove_if(m_observers.begin(), m_observers.end(), [this](Observer_reference iter) -> bool { Observer_strong_reference o = iter.lock(); return !o; } ), m_observers.end()); } public: void RegisterObserver(Observer_strong_reference& observer) { if (!observer) return; for (auto& iter : m_observers) { if (observer == iter.lock()) return; } m_observers.push_back(Observer_reference(observer)); } /*virtual*/ void OnChange(const Class_t* what) { bool hasZombies = false; for (auto& iter : m_observers) { Observer_strong_reference o = iter.lock(); if (o) { o->OnChange(what); } else { hasZombies = true; } } if (hasZombies) CleanupZombies(); } }; class CObservableUint32 : public CObservable<CObservableUint32> { uint32_t m_value; public: void Set(uint32_t newValue) { bool changed = newValue != m_value; m_value = newValue; if (changed) { OnChange(this); } } uint32_t Get() const { return m_value; } }; class CSomeObserver : public IObserver < CObservableUint32 > { public: CSomeObserver() : IObserver<CObservableUint32>() { } virtual ~CSomeObserver() { } virtual void OnChange(const CObservableUint32* what) { } }; 

Somewhere else...

  CObservableUint32 observable; { std::shared_ptr<IObserver<CObservableUint32> > observer = std::make_shared<CSomeObserver>(); observable.RegisterObserver(observer); observable.Set(42UL); } observable.Set(100);