Vector deallocating: COM troubles

[anonymous]

I have a pretty basic class, "Cube", that has a COM pointer member. The default constructor initializes this pointer using the correct API functions, the copy constructor copies the pointer and calls AddRef(). The destructor calls Release(). An abridged version of the code:
[code=cpp]
class Cube {
public:
Cube(...) {
_x = CreateObj(...); //Obtains a (new) valid COM object pointer
}

Cube(const Cube &src) {
_x = src._x;
_x->AddRef();
}

~Cube() {
if (_x) {
_x->Release();
_x = 0;
}
}

protected:
IObj* _x; //A COM object
};[/code]

Next, I create a vector of these "Cubes" and allocate them using the function below (simplified):
[code=cpp]//Global
vector<Cube> arr;

void Init() {
Cube* cube;

for (int i=0; i<10; i++) {
cube = new Cube(...); //Obtains a (new) valid COM object pointer
arr.push_back(* cube);
}
}[/code]

Now, in my cleanup function, I iterate through the vector and delete the objects. However, as soon as I delete the first "Cube", all members of the vectors are deallocated (filled with 0xfeeefeee when running through the debugger). Code:

[code=cpp]void Cleanup() {
for (vector<Cube>:: iterator i = arr.begin(); i < arr.end(); i++) {
delete &(*i);
}
}[/code]

As soon as the delete line is called the second time my app crashes.

EDIT: FYI, all the objects are valid, working and unique during the execution (between the calls to Init() and Cleanup()).

[weaknessforcats]

void Cleanup() {
for (vector<Cube>:: iterator i = arr.begin(); i < arr.end(); i++) {
delete &(*i);
}
}

You have an IObj* in your class. Your copy constructor just copies the pointer. Then when you Release() in your destructor, you release the single COM object and scrogg everything else in the vector.

You have a decision to make: Is there a COM object for each Cube or do all the Cube objects share the same COM object??

If each Cube needs its own IObj object, then you need to create one on your copy constructor.

If all Cubes share the same IObj, then you can't Release() until you are usre you have the last pointer. Here you use a Handle object (smart pointer). There is an article about this in the C/C++ articles section. In that article is a working template you can use in your own code.

[anonymous]

Thanks for the answer, but I do not think that's the problem. In Init(), each unique Cube is created by an API function which obtains a new object and point to it. The copy constructor is called (with AddRef() to keep the COM from being destroyed when the local goes out of scope) only in Init() when the data is copied from the local cube to the ones in the vector. During execution each Cube has its own COM object which is unique and independent (verified) and all pointers point to different areas of memory as well.

[Cucumber]

Try coding the operator= inside your Cube class, and do the AddRef And Release stuff.

            if( this==&r )
              return *this;
           if(_x!=NULL)  _x->Release();
           _x=r._x;
           if( _x!=NULL)  _x->AddRef();
           return *this;

[weaknessforcats]

Cube(const Cube &src) {
_x = src._x;
_x->AddRef();
}

This code does not create a new COM object. It just AddRef's the one that is in the Cube beiong copied.

if( this==&r )
return *this;
if(_x!=NULL) _x->Release();
_x=r._x;
if( _x!=NULL) _x->AddRef();
return *this;

This proposed assignment operator does not create a new COM object for the LVAL Cube. It just AddRef's the current COM object.

So I don't see how each Cube has it's own COM object.

Keep in mind that vector moves objects around by using the copy constructor and assignment operator every time it resizes the array.

This code:

for (int i=0; i<10; i++) {
cube = new Cube(...); //Obtains a (new) valid COM object pointer
arr.push_back(* cube);
}

Does not put the object created by new in the vector. Instead, a copy is made for the push_back call and this copy is copied again as it is placed in the vector array. When you return from push_back, the Cube destructor is called. The Cube* cube is overstored in the loop so the Cube object created by new leaks. There is no explicit deletion of it.

Also, is this program running in a MULTITHREADED_A PARTMENT? If so, you need critical sections around those AddRef's and Releases.

[weaknessforcats]

Also:

void Cleanup() {
for (vector<Cube>:: iterator i = arr.begin(); i < arr.end(); i++) {
delete &(*i);
}
}

doesn't work. The vector has Cube objects that were not allocated by the new operator. You can't delete objects you didn't create.

If this were a vector<Cube*>, then you may still jave a problem until you start using Handle<Cube> in your vector. (see Handle class article in ithe C/C++ Articles forum.

[anonymous]

This code does not create a new COM object. It just AddRef's the one that is in the Cube beiong copied.

That's the intended behaviour.

So I don't see how each Cube has it's own COM object.

Each object is created by the "new" operator in Init(), then copied and destroyed (Release()ing and thus decrementing the COM handle's reference count).

Keep in mind that vector moves objects around by using the copy constructor and assignment operator every time it resizes the array.

This is why I wrote the copy constructor. Gotta check if adding an assignment operator fixes it.

Does not put the object created by new in the vector. Instead, a copy is made for the push_back call and this copy is copied again as it is placed in the vector array.

Yes, it AddRef()s on each copy and Release()s on each destruction leaving one handle per object as far as I understand. However, as I understand it, the object is copied once from *cube to the vector, not twice.

The Cube* cube is overstored in the loop so the Cube object created by new leaks. There is no explicit deletion of it.

I tried deleting it explicitly after the push_back(), but the compiled code behaved the exact same way.

Also, is this program running in a MULTITHREADED_A PARTMENT? If so, you need critical sections around those AddRef's and Releases.

No, it's a simple single-threaded app. The code is as simplified as I could get it and still reproduce the behaviour anyway.

...doesn't work. The vector has Cube objects that were not allocated by the new operator. You can't delete objects you didn't create.

Well, I assumed the objects were created by new then copied. In any case, using vector::clear() does not call the destructor or release the handles. Not sure how I should destroy these objects if not by delete.

If this were a vector<Cube*>, then you may still jave a problem until you start using Handle<Cube> in your vector. (see Handle class article in ithe C/C++ Articles forum.

I'll look into that, but I'd rather find what's wrong with this code first.

[Cucumber]

What the OP is trying to achieve is basically wrapping a COM object inside a C++ class, which is pretty much the same as what ATL does with templates.

If I remember correctly, last time I took a look into the ATL code, the C++ templates that wrap COM objects only make a copy the COM pointer inside the Copy Constructor and the assignement operator, doing AddRefs and a Releases where appropiate. ATL does not generate a new COM object neither within the copy constructor nor within the assignement operator.

Now creating a perfect copy of the COM object each time you use the Copy Constructor or the Assignement Operator is possible, but you would have to implement a Clone method inside the COM object, otherwise the new COM object created inside the Copy Constructor or the Assignement Operator would not be the same as the one tried to be copied; it would be a blank-just-recently-initialized COM object and would not be the same as the already-used COM object you were trying to do a copy from.

By the way, to clear the vector just do an
arr.clear()

And your code is leaking memory, after calling the new operator you never deallocate the memory. You must do:

   Cube * cube = new Cube();  //<---- Allocates heap memory
  arr.push_back( *cube );  //<----- STL will create a copy of your C++ object so dont worry about your COM object being lost
  delete cube;   //<--- deallocate memory!!