'vector iterators incompatible'

Question

This question has been asked a number of times on SO, but the answers do not apply to my situation, AFAICT. The following piece of code is triggering the error immediately upon hitting i != std::end(observers_);.

void VisualGeometry::SignalPopPointFlags(const Point_2r& p,
                                         const uint32_t msec_delay) const {
    for(auto i = std::begin(observers_); i != std::end(observers_); ++i)
        (*i)->SlotPopPointFlags(p, msec_delay);
}

Looking into <vector>, the following triggers the error:

void _Compat(const _Myiter& _Right) const
{   // test for compatible iterator pair
    if (this->_Getcont() == 0
        || this->_Getcont() != _Right._Getcont())
        {   // report error
            _DEBUG_ERROR("vector iterators incompatible");
            _SCL_SECURE_INVALID_ARGUMENT;
        }
}

Since I am not comparing iterators from different containers, it seems like the first check for this->_Getcont() == 0 might be the problem, but I'm not sure how to tell.

The same problem occurs if I swap out begin(vec)/end(vec) for vec.begin()/vec.end().

I'm a little lost as to how this can happen. Any advice on how to move forward with debugging this?

The VisualGeometry class is designed to forward signals that it receives onward to whichever objects are watching it. Here are relevant code snippets:

class VisualGeometry : public IGeometryObserver, public IObservableGeometry {
public:
    void SlotPushSegmentFlags(const Segment_2r& s, const uint32_t flags,
                              const uint32_t msec_delay = 0) override;
    void SlotPopSegmentFlags(const Segment_2r& s,
                             const uint32_t msec_delay = 0) override;
    void SignalPushSegmentFlags(const Segment_2r& s, const uint32_t flags,
                                const uint32_t msec_delay = 0) const override;
    void SignalPopSegmentFlags(const Segment_2r& s,
                               const uint32_t msec_delay = 0) const override;
    /* snip */    
private:
    std::vector<IGeometryObserver*> observers_;
};

void VisualGeometry::SlotPushSegmentFlags(const Segment_2r& s,
                                          const uint32_t flags,
                                          const uint32_t msec_delay) {
    SignalPushSegmentFlags(s, flags, msec_delay);
}

void VisualGeometry::SlotPopPointFlags(const Point_2r& p,
                                       const uint32_t msec_delay) {
    SignalPopPointFlags(p, msec_delay);
}

/* etc... */

Answer 1

The first thing to check is to see if you are modifying the vector you are iterating over as you iterate over it.

See if this eliminates the problem:

void VisualGeometry::SignalPopPointFlags(const Point_2r& p,
                                         const uint32_t msec_delay) const {
  auto obs = observers_;
  for(auto i = std::begin(obs); i != std::end(obs); ++i)
    (*i)->SlotPopPointFlags(p, msec_delay);
}

Dealing with that kind of problem is tricky, and often a sign you have a design problem.

In general, when calling a sequence of callbacks, if that callback has any way to reach the sequence you are iterating over and change it or its members, you need to add in some lifetime management code, and determine what it means for another callback to be sent when one is currently being sent, and what it means if callbacks are installed or uninstalled while callbacks are being called.

A simple rule is that "you don't get callbacks if you are installed while callbacks are being installed", but if you uninstall your callback, you will not get called.

To produce this effect, my callbacks tend to be containers of weak_ptrs to std::functions. When you install your callback, you pass in a std::function, which I then copy into a shared_ptr. I spawn a weak_ptr off that, and store it in my callback container.

I then return the shared_ptr to the code that is installing the callback. This shared_ptr is the lifetime token: so long as it (or copies of it) is valid, I will continue to make callbacks on it.

In my broadcast function, I first sweep my container for obsolete weak_ptrs, then copy the container to a local std::vector<std::weak_ptr<std::function<void(message)>>>.

I then iterate over this container, doing a .lock() to get a std::shared_ptr<std::function<void(message)>>, then if it is valid calling it.

If a broadcast is triggered when I'm broadcasting, it happens. (if this happens too often, I'll blow my stack, but that is another problem). If someone adds a callback, I'm fine. If someone removes a callback, I'm fine.

If things are multi-threaded, I am not locked when iterating over the local std::vector, but am locked while clearing the weak_ptrs that aren't valid, and when cloning the sequence of weak_ptrs, or when adding/removing callbacks to the vector<weak_ptr<function<...>>>.