Invalid write size of 4 with matrix class (using valgrind)

I have a simple matrix class I've made for use with opengl (es 2.0, afaik there isn't any built in version for that particular version of opengl).

It is basically nothing more than a vector which is resized to contain 16 floats in the constructor, (I chose a vector over just a 16 float array for the = operator it had), and some convenience functions that are useful in the context of opengl. Anyway, one of these convenience structures loads the identity matrix into the vector, it is:

void Matrix::loadIdentity()
{
    matrix.resize(16, 0);
    for(int i = 0; i < 16; i++)
        matrix[i] = 0.0f;
    for(int i = 0; i < 4; i++) {
        matrix[i * 4 + i] = 1.0f;
    }
}

Where matrix is:

std::vector<float>

The line that says:

matrix.resize(16, 0);

is there to make sure that the size is actually 16 floats, although it's also done in the constructor, and I don't believe it will be needed ultimately, but right now to remove one possible bug, to make life easier for debugging.

Valgrind is telling me that this function, in particular this line:

matrix[i] = 0.0f;

is causing an invalid write size of 4, which sounds suspiciously like I'm somehow calling matrix[16] = 0.0f;. However, i should only go from 0 to 15. One interesting thing to note though is that valgrind also says that the memory in question that's getting written to, was allocated by the destructor of the matrix, and more specifically, the destructor of the vector.

Address 0x150f8280 is 0 bytes inside a block of size 64 free'd  1: operator delete(void*) in /build/buildd/valgrind-3.6.1/coregrind/m_replacemalloc/vg_replace_malloc.c:387
  2: __gnu_cxx::new_allocator&lt;float&gt;::deallocate(float*, unsigned long) in <a href="file:///usr/include/c++/4.5/ext/new_allocator.h:95" >/usr/include/c++/4.5/ext/new_allocator.h:95</a>
  3: std::_Vector_base&lt;float, std::allocator&lt;float&gt; &gt;::_M_deallocate(float*, unsigned long) in <a href="file:///usr/include/c++/4.5/bits/stl_vector.h:146" >/usr/include/c++/4.5/bits/stl_vector.h:146</a>
  4: std::_Vector_base&lt;float, std::allocator&lt;float&gt; &gt;::~_Vector_base() in <a href="file:///usr/include/c++/4.5/bits/stl_vector.h:132" >/usr/include/c++/4.5/bits/stl_vector.h:132</a>
  5: std::vector&lt;float, std::allocator&lt;float&gt; &gt;::~vector() in <a href="file:///usr/include/c++/4.5/bits/stl_vector.h:314" >/usr/include/c++/4.5/bits/stl_vector.h:314</a>
  6: Matrix::~Matrix() in <a href="file:///home/leif/MarbleMachine/core/matrix.h:6" >core/matrix.h:6</a>

This seems unlikely if it's just some sort of index out of range error, seeing as, afaik, a vector object is primarily a pointer to where the array is in memory (well, there's other stuff too, but the point I'm trying to make is that the vector object itself (where the destructor would be stored), seems unlikely that it would be contiguous in memory with where the actual array is stored. Also, on top of that, why would a destructor claim memory when it's whole point is to free up the memory an object uses, wouldn't that just create a memory leak? By the way, the matrix class doesn't have an explicit destructor, so just the implicit one is used.

Finally, I know that previous errors in valgrind can cause future errors, but the only previous errors is invalid read size and use of uninitiated variables inside of existing libraries (OpenGL and SDL), as such, I find it extremely unlikely that they are causing a heap corruption before this happens.

Thank you.

Solution

It's pretty clear from the snippet of Valgrind report you posted that you are accessing dangling storage for a vector.

Such access could easily arise if one thread is iterating over a vector, while another thread has resized it. For example, this is unsafe:

for (vector<float>::iterator it = matrix.begin(); it != matrix.end(); ++it)
  *it = 0;

while in another thread:

matrix.resize(32, 0);

If you have mutators that can resize the vector, then you must serialize all readers with respect to them, e.g.

pthread_mutex_lock(&matrix_lock);
for (vector<float>::iterator it = matrix.begin(); it != matrix.end(); ++it)
  *it = 0;
pthread_mutex_unlock(&matrix_lock);

In another thread:

pthread_mutex_lock(&matrix_lock);
matrix.resize(32, 0);
pthread_mutex_unlock(&matrix_lock);

What you've described in your answer:

1 Graphics thread gets matrix.end() outside of lock [which is already a problem].
...
6 Graphics thread locks
7 Graphics thread gets matrix.begin() and starts iterating

appears to not be consistent with Valgrind error report, because it would have resulted in access past the end of the array, whereas Valgrind reports access at the beginning of the (now deleted/dangling array).

Bottom line: calling begin(), end(), and iteration must all happen within a lock that protects the container; otherwise your program behavior will be undefined.