So I have this function to add listeners and it converts shared pointers of a class so that I can call it 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 have purposely not created ANY threads (that I know of) so I am just super confused what I am doing wrong to make these weak_ptrs break. So if I run it in gdb, it works just fine. But if I set, "set disable-randomization off" then I get the error. So I feel like this is some weird problem where with uninitialized variables. If it helps, this is the stack when I crash in 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 the possibility of listeners getting registered while new listeners where being added, which could totally happen. This would cause the iterator to be invalid, and when I go to call weak_ptr.lock() on a section of memory that isn't a weak_ptr, BOOM. I think there is a moral in here somewhere I just have to find it.
When notify() is called is it possible that the function called through the onNotify() function object will result in registerListener() being called indirectly (or some other code that can add or remove entries in the listeners collection)?
If so, then the iter being used in the notify() for loop can be invalidated. You might want to change notify() to look something like the following which queues up all the shared_ptr objects to notify so that it doesn't matter if the listeners collection is modified during any of the onNotify() callbacks:
#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();
}
}