Is this matrix class thread safe when writing on different indices?

Question

As you may notice there is a double* storage behind this. Any value read/written is translated into a flat index so it is stored in a large single dimentional array.

I need to know whether i can write on different indices in an object of this type in different threads? I am using the operator () to set the values. The bounds are predefined when intantiating the object. i.e: Matrix m(rows,cols); I am using this in boost threads and having issues. Do i need to put any scoped locks? but that would be a performance cost i believe.

class matrix
{
public:
    matrix(size_t rows, size_t cols):_rows(rows),_cols(cols),_data(0)
    {
        reset(rows,  cols);
    }
    matrix():_data(0),_rows(0),_cols(0){}
    ~matrix()
    {
        if (0 != _data)
        {
            delete [] _data;
        }
    }

    matrix(const matrix& copythis)
        :_data(0),_rows(0),_cols(0)
    {
        if (0 != _data)
        {
            delete [] _data;
        }
        _rows = copythis.rows();
        _cols = copythis.cols();
        _data = new double [_rows * _cols];

        if (0 == _data)
        {
            NL_THROW("Insufficient memory to create a cloned matrix of double of " << _rows << " X " << _cols);
        }

        memcpy(_data, copythis._data, _rows * _cols * sizeof(double) );

    }


public:

    const matrix& operator = (const matrix& copythis)
    {
        if (0 != _data)
        {
            delete [] _data;
        }
        _rows = copythis.rows();
        _cols = copythis.cols();
        _data = new double [_rows * _cols];

        if (0 == _data)
        {
            NL_THROW("Insufficient memory to create a cloned matrix of double of " << _rows << " X " << _cols);
        }

        memcpy(_data, copythis._data, _rows * _cols * sizeof(double) );

        return (*this);

    }







    double* cArray()
    {
        if (0 == _data)
        {
            NL_THROW("Matrix is not initialised");
        }
        return _data;
    }



    void reset(size_t rows, size_t cols)
    {
        if (0 != _data)
        {
            delete [] _data;
        }
        _rows = rows;
        _cols = cols;
        _data = new double[rows * cols];

        if (0 == _data)
        {
            NL_THROW("Insufficient memory to create a matrix of double of " << _rows << " X " << _cols);
        }
        memset(_data, 0, sizeof(double) * _rows * _cols);
    }





    double& operator () (size_t rowIndex, size_t colIndex) const
    {
        if (rowIndex >= _rows)
        {
            NL_THROW("Row index " << rowIndex << " out of range(" << _rows - 1 << ")");
        }

        if (colIndex >= _cols)
        {
            NL_THROW("Column index " << colIndex << " out of range(" << _cols - 1 << ")");
        }

        size_t flatIndex = colIndex + rowIndex * _cols;
        return _data[flatIndex];

    }


    Array rowSlice(size_t rowIndex) const
    {
        if (rowIndex >= _rows)
        {
            NL_THROW("Cannot slice matrix, required row: " << rowIndex << " is out of range(" << (_rows - 1) << ")");
        }
        Array retval(_data + rowIndex * _cols, _cols);

        /*
        for(size_t i = 0; i < _cols; i++)
        {
            retval[i] = operator()(rowIndex, i);
        }
        */
        return retval;
    }



    Array colSlice(size_t colIndex) const
    {
        if (colIndex >= _cols)
        {
            NL_THROW("Cannot slice matrix, required row: " << colIndex << " is out of range(" << (_cols - 1) << ")");
        }
        Array retval(_rows);
        for(size_t i = 0; i < _rows; i++)
        {
            retval[i] = operator()(i, colIndex);
        }
        return retval;
    }


    void fill(double value)
    {
        for(size_t rowIndex = 0; rowIndex < _rows; rowIndex++)
        {
            for(size_t colIndex = 0; colIndex < _cols; colIndex++)
            {
                size_t flatIndex = colIndex + rowIndex * _cols;
                _data[flatIndex] = value;
            }
        }
    }


    bool isEmpty() const
    {
        if (0 == _rows) return true;
        if (0 == _cols) return true;
        return false;
    }

    size_t rows() const {return _rows;}
    size_t cols() const {return _cols;}

private:
    double* _data;
    size_t _rows;
    size_t _cols;
};

Answer 1

The way this class is written, you need locks on every method except the getters rows() and cols(), pretty much at the broadest possible scope on each of those methods.

You can't even rely on the size_t type being atomic from saving you any locks, because reset() sets both _row and _col non-atomically. Any operation that needs to know about _row and _col, e.g. isEmpty(), will run into trouble if reset() is called from another thread.

Answer 2

If you can guarantee that all your threads never collide on the same element of the matrix, then you won't need locking. In practice it's not realistic to expect such a guarantee, so you'll need locking.