CODEWITHSUNDEEP
c++vectordestructor
Alex Shul
Alex Shul

Reputation: 500

Destructor is called when I push_back to the vector

I have this class definition:

class FlashStream
{
public:
    explicit FlashStream(const char * url, vector<uint8> * headers, vector<uint8> * data, void * ndata, void * notifyData = NULL, uint32 lastModified = NULL);
    ~FlashStream();
private:        
    NPStream      _stream;
    // ...
}

(NPStream description)

and its implemetation:

FlashStream::FlashStream(const char * url, vector<uint8> * headers, vector<uint8> * data, void * ndata, void * notifyData, uint32 lastModified)
{
    // ...
    memset(&_stream, 0, sizeof(NPStream));

    _stream.headers = new char[data->size()]; 

    memcpy((void*)_stream.headers, &(*data)[0], data->size());
    // ...
}

FlashStream::~FlashStream()
{
    // ...
    if(_stream.headers)
        delete [] _stream.headers;
    _stream.headers = NULL;
    // ...
}

Now, when I run this code:

// ...
vector<FlashStream> _streams;
// ...
_streams.push_back(FlashStream(url, headers, data, _npp.ndata, notifyData, lastModified));
// ...

Sometimes I have an error at delete [] _stream.headers; in the destructor of FlashStream, which is called when I push_back() to the vector<FlashStream> _streams.

I read this question on SO and a few another, but all the same don't know how to elegantly and efficiently fix the problem. May be the problem is in copy constructor, but I don't know how I can make it with memory allocation for NPStream.headers and NPStream.url?

Upvotes: 5

Views: 4636

Answers (4)

ivaigult
ivaigult

Reputation: 6687

Destructor at push_back() could be called in two cases.

  • First case, as was noted, occurs when you push a temporary object to the vector. This is not the best idea, since the constructor, copy constructor, and destructor will be called. You may store pointers to object in your vector to avoid redundant calls.

    C++11 comes with new features that might help you.

    • To avoid unnecessary copying, use std::vector::emplace_back(). It constructs an object in-place in the vector, instead of copying.

    • Also, you may use the move version of push_back(value_type&& val). Just define a move constructor in your class, and the move version of push_back() will work automatically for temporary objects.

  • The second case is reaching capacity of the vector. A vector has two major values: a size and a capacity. The size is the number of elements currently held in the vector. The capacity is the number of elements the vector storage can hold. So, when you push back an element, you are increasing the size of the vector. If the capacity is not large enough to hold the new element, the vector performs reallocation to increase its capacity. During reallocation, the vector re-constructs its objects using the copy/move constructor and deletes old objects using the destructor. So, at push_back(), the vector could invoke the destructor of the objects multiple times.

To decrease the cost of vector re-sizes at push_back(), always use the std::vector::reserve() method to pre-allocate the storage for your objects:

std::vector<int> vec;
vec.reserve(20);
for(int i = 0; i<20; ++i)
    vec.push_back(i)

Also, you may decrease the cost of copying objects by defining a move constructor:

class C
{
public:
    C(int c)
        : m_c(c)
    {
        std::cout << "C(int c)" << std::endl;
    }
    C(C&& c)
        : m_c(c.m_c)
    {
        std::cout << "C(C&& c)" << std::endl;
    }
    C(const C& c)
        : m_c(c.m_c)
    {
        std::cout << "C(const C& c)" << std::endl;
    }
    ~C()
    {
        std::cout << "~C()" << std::endl;
    }
private:
    int m_c;
};

int main()
{
    std::vector<C> vc;
    for (int i = 0; i < 100; ++i)
        vc.push_back(C(i));
    return 0;
}

If you compile and run it, you will see that C(const C& c) was not called at all. Since a move constructor is defined, push_back() and reallocation will move your objects instead of copying them.

Upvotes: 6

Nipun Talukdar
Nipun Talukdar

Reputation: 5387

Putting copies of objects in vector/map etc. are bad idea. The destructor called when the temporary objects get destroyed. The destructor for the objects will be again called whenever the Vector/Map resizes or rearranges.

To avoid those you should store pointer to those objects. You may like to use shared_ptrs here.

Upvotes: 0

Martin J.
Martin J.

Reputation: 5118

This statement:

_streams.push_back(FlashStream(url, headers, data, _npp.ndata, notifyData, lastModified));

is equivalent to:

{
    FlashStream temp(url, headers, data, _npp.ndata, notifyData, lastModified);
    _streams.push_back(temp);
    // temp gets destroyed here
}

so, you create a temporary FlashStream object that is copied into the vector, then destructed afterwards. You can avoid this by using emplace_back() in C++11:

_streams.emplace_back(url, headers, data, _npp.ndata, notifyData, lastModified);

Upvotes: 5

Idov
Idov

Reputation: 5124

You're creating a temporary object when you push back.
He's the one which its desctructor is called.

Upvotes: 0

Related Questions