C++: Vector of Pointers to Objects from another Vector

Question

I have two classes, similar to this:

class A
{
public:
    B* ptr1;
}

class B
{
public:
    std::vector<A*> list;
}

In the main implementation, I'm doing something like this:

int main() {

// there are a lot more A objects than B objects, i.e. listOfA.size() >>> listOfB.size()
std::vector<A> listOfA;
std::vector<B> listOfB; 

while (//some loop)
{
    listOfB[jj].list.push_back( &(listofA[ii]) );
    listOfA[ii].ptr1 = &( listOfB[jj] );
}

} // int main end

Basically something like this. A lot of A objects are assigned to one B object, and these A objects are stored in that pointer vector as pointers. Additionally, each of these A objects get a pointer to the B object they belong to. For the context, I'm basically doing an Connected Components Algorithm with run-length-encoding (for image segmentation), where class A are the line segments and class B are the final objects in the image.

So, the pointers of the vector in class B all point to Objects which are stored in a regular vector. These objects should be deleted when the regular vector goes out of scope, right? I've read that a vector of pointer like in class B usually requires writing a manual destructor, but this shouldn't be the case here, I think...

The reason why I'm asking is, of course, because my code keeps crashing. I'm using an Asus Xtion Pro camera to get the images and then am performing the algorithm on every image. Weird thing is, the program crashes whenever I shake the camera a bit harder. When the camera is stationary or moved only a little or slowly, nothing happens. Also, when I use a different algorithm (also connected components, but without run-length-encoding and also doesn't use pointers), nothing crashes, no matter how much I shake the camera. Also, in Debug mode (which ran much slower than the Release mode), nothing crashed either.

I tried making a destructor for the pointer vector in class B, but it resulted in a "block is valid" error, so I guess it deleted something twice. I also tried replacing every pointer wih a c++11 std::shared_ptr, but that only produced very irregular behaviour and the code still crashed when I shaked the camera.

I basically just want to know if in terms of memory leaking and pointer handling, the code shown above seems fine or if there are mistakes in the code which could lead to crashes.

EDIT (SOLVED): The solution (see the accepted answer) was to ensure that the vector 'listOfB' doesn't get resized during run-time, for example by using 'reserve()' to reserve enough space for it. After doing this, everything worked fine! Apparently it worked, because if the vector 'listOfB' gets resized (by push_back()), the internal memory adresses of the B instances in it are also changed, causing the pointers in the A instances (which point to B instances) to now point to the wrong adresses - and thus resulting in trouble, which lead to the crash.

About the camera shaking, apparently, shaking the camera resulted in very blurry pictures with lot of elements to segment, thus increasing the number of objects (i.e., resulting in higher size required for listOfB). So, mystery solved! Thanks a lot! :-)

Answer 1

I think the design is broken. listofB will grow (you do push_backs) and re-allocate its internal data array, invalidating all addresses stored in the ptrs of the A instances. The usual algorithm will grow the data size by a factor of 2 which may explain that you are good for a while if not too much data arrives. Also, as long as the memory of the old data is still in the address space of the program (especially if it is on the same memory page, for example because the new data fits in it as well), the program may not crash accessing it and just retrieve old data.

On a more constructive note: Your solution will work if you know the maximum elements in advance, which may be hard (think you get a 4k camera next year ;-)). In that case you can, by the way, just take a simple static array anyway.

Perhaps you could also use a std::map to store A objects instead of a simple vector listofA. Each A object would need a unique ID of some sort (static counter in A in the easiest case) to use as a key into the map. Bs would store keys, not addresses of As.

Answer 2

Assuming you have not made a mistake in how you build your network you should be fine. You would have to post more code to assess that. Also you can not use either of the vectors after you change one of them because if they reallocate their members all pointers pointing to them are invalidated. But using raw pointers to managed objects is the correct way to build networks.

By managed objects I mean objects whose lifetime is guaranteed to last longer than the network and whose memory will be automatically released. So they should be elements of a container or objects managed by some kind of smart pointer.

However it sounds like you have a hardware problem.