3

Whenever a vector of my objects gets reallocated, the object's destructor is called and it's causing me problems.

struct Object
    {
        Object(int buffer_ID) : buffer_ID(buffer_ID){ OpenGLCreateBuffer(buffer_ID, somedata);}
        ~Object() { OpenGLDeleteBuffer(buffer_ID); }
        int buffer_ID;
    };

    int main()
    {
        std::vector<Object> objArr;
        objArr.push_back(1);
        objArr.push_back(2);             // Destructor is called when reallocating vector

        OpenGLDrawBuffer(objArr[0].buffer_ID);       // The buffer has been deleted

    }

This is really annoying, I don't understand why the destructor is called on an object that's being moved. I've looked around and I'm pretty satisfied that you can't STOP the destructor from being called. With move semantics I think the trick utilised is to copy the contents from the other object, and set any pointers to null, so that when the destructor is called on them and the resources released, delete is just being called on a nullptr.

I first tried to make a copy constructor, and tried to set the other buffer_ID to 0, but this only works if the copy constructor takes a non-const reference, which doesn't seem right. Also, the act of setting the other's variables to null so that delete is then called on null, or null passed to something like in this case OpenGL delete function, seems hacky, doesn't it?

I know I'm going to be told that I can't stop the destructor from being called, so what should I do in the case where the object may get reallocated to another part? The destructor is the best place to be deleting stuff like this I think.

Thanks.

Zebrafish
  • 11,682
  • 3
  • 43
  • 119

3 Answers3

6

Your class appears to violate the Rule Of Three. Its destructor appears to destroy a resource that wasn't created in its constructor.

Your real problem is that the fundamental design of your class is incompatible with how std::vector works. When a vector has to reallocate its contents, its going to either copy-construct or move-construct (if the vector's class support move semantics) the existing instances, then destroy all the old class instances. That's how a vector works. That's how it's designed. If your class cannot work like that, you cannot use std::vector. Using std::list would be one option, perhaps.

But a better option is to fix your class. Redesign so that it complies with the Rule Of Three. After doing that, further extend your class to add a move constructor, and proper support for move semantics.

Community
  • 1
  • 1
Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • The buffer_ID that's deleted in the destructor is created in the constructor, I just haven't shown it, sorry, I left that bit out in the illustration. You're right, vector probably isn't for me. If I were to make my own vector class it probably wouldn't call destructor when moving. – Zebrafish Jan 26 '17 at 02:51
  • 1
    If so, then your vector wouldn't be C++. It is fundamental to C++ that every class instance's creation must result in a constructor call, and every class instance's destruction must result in a destructor call. No exceptions. Move semantics, introduced with C++11, are precisely what you're looking for. They were introduced precisely for the reason of implementing an efficient way of moving an object, while still keeping C++'s fundamental ways of using objects intact. – Sam Varshavchik Jan 26 '17 at 03:22
0

I ran into the same problem earlier today. Took me a while to realize what was happening.

I fixed it by making a vector of shared pointers to openGl objects instead

-1

You can use mutable to get around the const problem in the copy constructor. Why not add a release method to the incoming class to zero out the pointer.

london-deveoper
  • 533
  • 3
  • 8
  • In the original post "but this only works if the copy constructor takes a non-const reference". Also re. code the OP referred to deletion of a pointer in their post. – london-deveoper Jan 26 '17 at 01:37
  • Yep, sorry, I re-read the question and saw where you were coming from. I still don't think this answers the question though. `mutable` (or `const_cast`) aren't the correct solution. A move constructor and correct ownership semantics are. – Miles Budnek Jan 26 '17 at 01:39
  • Read my previous comment again. OP doesn't need `mutable` or any sort of release method. OP needs a move constructor. – Miles Budnek Jan 26 '17 at 05:04