0

I have the following code in an application that threw an access violation exception:

size_t CConnectionsDoc::get_active_connections( std::vector<CString> &conn )
{
    CString temp;
    size_t cnt = 0;
    conn.clear();

    if( initialized ) {
        for( std::vector<ACTIVE_CONNECTIONS>::const_iterator c_i = connections_vector.begin();
                c_i != connections_vector.end(); c_i++ ) {
            temp.Format( "%s:%d:%d:%lu", ( LPCTSTR )c_i->their_ip,
                         c_i->their_port, c_i->our_sd, c_i->their_pa );
            conn.push_back( temp );
            cnt++;
        }
    }

    return cnt;
    }


void CConnectionsDoc::update_connections( const uint sd )
{
    std::vector<ACTIVE_CONNECTIONS>::iterator iter = connections_vector.begin();

    while( iter != connections_vector.end() ) {
        if( iter->our_sd == sd ) {
            connections_vector.erase(iter);
            break;
        }

        iter++;
    }
}

typedef struct active_connections
{
    CString their_ip;
    unsigned int their_port;
    unsigned int our_sd;
    unsigned long their_pa;
} ACTIVE_CONNECTIONS;

example data
    their_ip  "192.168.1.125"
    their_port 60849
    our_sd     1096
    their_pa   2097260736

This is a Visual Studio 2012 C++ app and from a debugging session using a dump file I found the following values:

initialized=1
connections_vector size=8
connections_vector capacity=13
connections_vector entries 0-7 have valid data and debugger does not show any entries past element 7
cnt=13 at the time of the crash (odd it is the same size of the capacity)
conn size=13
conn capacity=13

std::vector conn has the 8 correct entries from the connections_vector plus 5 entries that look like valid data, but connections_vector.erase(it) was called in another function to remove disconnected entries prior to calling get_active_connections.

It appears that the const_iterator went beyond connections_vector.end() until it tried to access one element beyond the capacity of the connections_vector and crashed, but I don't see how that is possible. Any ideas? Thanks in advance.

user1139053
  • 131
  • 3
  • 10
  • `std::vector conn has the 8 correct entries from the connections_vector plus 5 entries that look like valid data, but connections_vector.erase(it) was called in another function to remove disconnected entries prior to calling get_active_connections.` Time for you to post this code and exactly what you erased. – PaulMcKenzie Apr 16 '15 at 22:21
  • 2
    The posted code looks OK to me. @JonnyHenly, pre-increment won't make any difference here. – R Sahu Apr 16 '15 at 22:21
  • try the pre-increment operator in the step phase of your for loop. i.e.) change `c_i++` to `++c_i`. If that doesn't solve your problem then we'll need more information, preferably: what valid data looks like? the code to the function that called `connections_vector.erase(it)`. – Jonny Henly Apr 16 '15 at 22:21
  • c_i != connections_vector.end() - 1? – CantThinkOfAnything Apr 16 '15 at 22:23
  • 1
    `connections_vector.erase(it)` My crystal ball is saying that you used the "erase while looping over the container" method, and not the `remove_if/erase` idiom. – PaulMcKenzie Apr 16 '15 at 22:23
  • @RSahu I didn't think it would, but pre-increment is most likely to be faster but never slower than post-increment. Although off topic, it's still a positive improvement. – Jonny Henly Apr 16 '15 at 22:25
  • http://stackoverflow.com/help/how-to-ask "Not all questions benefit from including code. But don't just copy in your entire program! Not only is this likely to get you in trouble if you're posting your employer's code, it likely includes a lot of irrelevant details that readers will need to ignore when trying to reproduce the problem. Here are some guidelines: Include just enough code to allow others to reproduce the problem. For help with this, read How to create a [Minimal, Complete, and Verifiable](http://stackoverflow.com/help/mcve)" – Mooing Duck Apr 16 '15 at 23:04
  • I added the code that performs the vector erase and the structure of the elements in the vector. I posted an edit when I added this but for some reason my edit does not appear, only my additional code. – user1139053 Apr 16 '15 at 23:04
  • You aren't calling them from two different threads, are you? – Ishamael Apr 16 '15 at 23:05
  • No, all calls are from one thread. Thanks for the suggestion – user1139053 Apr 16 '15 at 23:09
  • Why did I get a down vote? Did I do something wrong? – user1139053 Apr 16 '15 at 23:11
  • You included a ton of irrelevant code in your question. The problem is very simple, once you pass an iterator to `erase`, the iterator is no longer an iterator into the container, so incrementing it won't work. – David Schwartz Apr 16 '15 at 23:58
  • @user1139053 My crystal ball is right. – PaulMcKenzie Apr 16 '15 at 23:58
  • @user1139053 If your goal is to erase the first item found that matches the criteria, then `std::find_if` should have been used instead of that loop. Second, do you have the same type of erasure code somewhere else in your program, where you do *not* break out of the loop, but instead keep looping? If so, and your loops look like the one you posted, then the loop is incorrect, as specified by the answer given. – PaulMcKenzie Apr 17 '15 at 00:40

1 Answers1

1

You tried to erase some of the objects from the same vector. If you don't use erase remove idiom, data will not be cleaned up from vector. On top of that you did erase operation inside a loop, so iterator is invalidated. Please refer following more details

Iterator invalidation rules

http://en.wikipedia.org/wiki/Erase–remove_idiom

Community
  • 1
  • 1
Steephen
  • 14,645
  • 7
  • 40
  • 47