7

I am writing a small UI for my program. I have the method onMouseMotion(), which I can call in one of two ways (see code); if I call it through std::function, then != operator in the for loop stop condition produces the run-time exception vector iterators incompatible. Why?

class Widget : public EventHandler
{
protected:
    /* ... */
    std::vector<Widget *> children_;                        
    std::function<bool(Event &)> func_;

private:
    bool onMouseMotion(Event &event);
    /* ... */   
};

Widget::Widget() 
{
    /* ... */
    func_ = std::bind(&Widget::onMouseMotion, this, std::placeholders::_1);
    /* ... */
}

bool Widget::processEvent(Event &event)
{
    if (event.getType() == ui::EventType::MouseMotionEvent) {

        /* Method 1 - onMouseMotion works ok */     
        onMouseMotion(event);

        /* Method 2 - onMouseMotion throws */
        //func_(event);

        return true;
    }
}

bool Widget::onMouseMotion(Event &event)
{
    /* exception occurs on the next line, only when using Method 2 above */
    for (auto child = children_.rbegin(); child != children_.rend(); ++child) {}
}

Updates:

  • program is single-threaded.
  • the exception is thrown when entering the for loop, zero iterations occur.
  • compiling with MSVC.
  • same exception with an empty for loop.
  • rewritten examples to illustrate the std::function problem.
Dan Nestor
  • 2,441
  • 1
  • 24
  • 45
  • Can `children_` vector be modified by something from another thread? If it can, than the answer is simple: vector reallocates and iterator becomes invalid. Use `std::list` instead of vector then. – Tsar Ioann May 29 '14 at 10:02
  • Nope, my program is single-threaded. – Dan Nestor May 29 '14 at 10:04
  • how many times is it looping before the exception is thrown ? – Moha the almighty camel May 29 '14 at 10:06
  • And `(*hovered_)->processEvent` does not modify the vector either? – Tsar Ioann May 29 '14 at 10:07
  • How is `howered_` initialized before the first call to `onMouseMotion`? If it is not, you cannot use it in comparisons without incurring UB. – ach May 29 '14 at 10:07
  • hovered_ is irrelevant, the program doesn't get there. – Dan Nestor May 29 '14 at 10:08
  • @Mhd.Tahawi zero times. The exception is thrown when entering the for loop. – Dan Nestor May 29 '14 at 10:12
  • did you try using a debugger, putting a break point at before you enter the loop and check the values of the iterators and length of the vector ? – Moha the almighty camel May 29 '14 at 10:17
  • @TsarIoann - no, it doesn't modify it. The vector is actually `protected` (I corrected the q.) – Dan Nestor May 29 '14 at 10:18
  • What happens if you replace `auto child` with `std::vector::reverse_iterator child = children_.rbegin();`? Probably irrelevant, but the line `hovered_ = child.base();` should have produced a compilation error, too. – Michael Foukarakis May 29 '14 at 10:18
  • @AndreyChernyakhovskiy - the program is not doing any comparisons with `hovered_` before throwing. – Dan Nestor May 29 '14 at 10:18
  • did you try removing `auto` and put the explicit type instead ? – Moha the almighty camel May 29 '14 at 10:19
  • same thing with explicit type instead of `auto` :( – Dan Nestor May 29 '14 at 10:20
  • @MichaelFoukarakis It should not have produced compilation error, cause conversion is done. See: http://stackoverflow.com/a/2037917/1565832 – Tsar Ioann May 29 '14 at 10:22
  • 3
    I think it's time to name the compiler that you are using. – CB Bailey May 29 '14 at 10:27
  • Good point, it's MSVC. – Dan Nestor May 29 '14 at 10:28
  • did you try using an index based loop instead of iterators ? (at least it is work around till you find out what is wrong ) – Moha the almighty camel May 29 '14 at 10:39
  • no, and I will stick with iterators because I want to find out what is wrong :D – Dan Nestor May 29 '14 at 10:40
  • Do you initialize hovered_ somewhere? – Paolo Brandoli May 29 '14 at 10:41
  • @PBrandoli: I guess it is irrelevant since the exception is being thrown before entering the body of the loop – Moha the almighty camel May 29 '14 at 10:43
  • Anyway, yes, `hovered_` is initialized to `children_.end()` in the class constructor. – Dan Nestor May 29 '14 at 10:45
  • I have heard of some bugs in MSVC when using [auto keyword](http://connect.microsoft.com/VisualStudio/feedback/details/779867/assertion-vector-iterators-incompatible-fails-when-using-auto-keywork-with-vector-iterator). For the time being you can replace auto with type required until you can find the root cause of the issue. – Mohit Jain May 29 '14 at 10:46
  • @MohitJain: he is getting the error even with the explict type – Moha the almighty camel May 29 '14 at 10:48
  • @DanNestor: would you please try to remove the body of the loop, throw a simple printing statement and try it, is it still throwing the exception like that ? – Moha the almighty camel May 29 '14 at 10:50
  • @Mhd.Tahawi Sorry I missed the conversation. Looked at the question just now. @Dan Does the same problem occur even if you use `bool onMouseMotion(Event &event) const;`? Can you remove the condition from `for loop` and print the [types](http://www.cplusplus.com/reference/typeinfo/type_info/name/) of `child` and `children_.rend()`? You would need to enable RTTI if not already enabled. – Mohit Jain May 29 '14 at 10:55
  • Same thing happenning with empty `for` loop body. – Dan Nestor May 29 '14 at 10:58
  • Is it possible that onMouseMotion is called on a Widget object that has been deallocated? – Paolo Brandoli May 29 '14 at 11:01
  • 1
    @DanNestor Can you [print the type](http://ideone.com/bMS85r) of `child` and `children_.rend()`? – Mohit Jain May 29 '14 at 11:02
  • `typeid.name()` is identical, namely `class std::reverse_iterator > > >` for both `child` and `children_.rend()`. – Dan Nestor May 29 '14 at 11:03
  • ok, I managed to eliminate the exception with a workaround, updating question now. – Dan Nestor May 29 '14 at 11:05
  • It may sound stupid, but can you use [std::rbegin and std::rend](http://en.cppreference.com/w/cpp/iterator/rbegin) – Mohit Jain May 29 '14 at 11:09
  • @DanNestor Which workaround did you use to make it work? – Paolo Brandoli May 29 '14 at 11:33
  • 1
    I once investigated similar 'mysterious' effect (a program ran perfectly well in Release but stumbled into the assertion in Debug). Eventually, I found out that the contents of object containing the container in question was `memset`'ted to 0 in the constructor. That destroyed its internal structures, and thence all iterators obtained from that container were considered by the run-time check procedures to be having 'singular values'. – ach May 29 '14 at 11:51
  • I updated the question. The problem only occurs if I call the method through a `std::function`. Any ideas? Am I using it wrong? – Dan Nestor May 29 '14 at 11:57
  • Run your program and when RTL shows you the assertion, click 'Retry' to switch to the debugger. The top stack frame is likely to be `std::_Debug_message`, switch to the frame immediately underneath it. You must then find yourself in `std::_Vector_const_iterator::_Compat`. Inspect `this->_Myproxy` and `this->_Myproxy->_Mycont`. They both must be not null, and the latter must point to your vector. If that is not so, you have somewhere caused corruption to your vector object. – ach May 29 '14 at 11:59
  • Does `EventHandler` inherit from `Event`? – Mohit Jain May 29 '14 at 12:03
  • @MohitJain no, it doesn't – Dan Nestor May 29 '14 at 12:04
  • @AndreyChernyakhovskiy looks like `child` is corrupt somehow, in my Watch window it says "unable to read memory". I wasn't able to find the frames you mentioned. – Dan Nestor May 29 '14 at 12:06
  • Could this be an access problem? By accessing my method through the functor, does the access level change? I'm thinking no, since `onMouseMotion` is private. – Dan Nestor May 29 '14 at 12:08
  • Please ignore my last comment, there was some confusion. Once you have given your address (to a functor or to anybody else), there is nothing like private, static etc. – Mohit Jain May 29 '14 at 12:09
  • [This works](http://ideone.com/kL0Lfo). So there is no restriction like public, private. – Mohit Jain May 29 '14 at 12:15
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/54722/discussion-between-dan-nestor-and-mohit-jain). – Dan Nestor May 29 '14 at 12:28
  • I found the problem - initializing an object with `obj({x, y})` instead of `obj(x, y)` cleared the error. I don't quite understand though why the compiler let me do that. – Dan Nestor May 29 '14 at 13:21
  • Your class does not seem to have a custom copy ctor, move ctor and assignment-operators. If there's any copy/move, the captured `this` might refer to another object. E.g. `auto w = Widget();` w/o copy/move elision – dyp May 29 '14 at 14:50

1 Answers1

3

So it is clear that

  • auto defines an iterator type for child, determined statically by the compiler (it cannot change between calls).
  • the type of child is assignment compatible with rbegin() and rend()
  • the type of child is relational operator compatible with rbegin() when called directly, but not when called through the bind() wrapper
  • since the type of child cannot change, the type of rend() must have, in the second case.

I see the following possibilities.

  1. The vector is a member and therefore the iterator type will be a pointer to member, and pointer to member has some restrictions on how it may be used.
  2. The value of this in the first case might be different from that captured in the bind (base class vs derived class, for example)
  3. The value of this may be delivered via a wrapper, which changes its type behaviour.

On balance, this is most likely to be an MSVC bug. The code as supplied does not compile and I am reluctant to try to modify and likely not be able to reproduce the bug. If you can post a repro case that compiles, I'd be happy to investigate further and update the answer.

david.pfx
  • 10,520
  • 3
  • 30
  • 63