Why heap corruption when returning an object Mat?

Question

I don't understand why I get a corrupted heap error with this program (I'm using OpenCV for the class Mat):

class A {
    private:
    Mat image;      

    static UINT ThreadProc( LPVOID pParam ) {
        A* pThis= (ClientNetwork*)pParam;
        UINT nRet= pThis->DoThreadProc();     // get out of 'static mode'
        return( nRet );
    }
    UINT ClientNetwork::DoThreadProc() {
         vector<uchar> vect;
         while(1) {
             /**** initialize vect and get the image data to decode ****/

             decode(vect);
         }
    }

    public:
    void decode(const vector<uchar>& vectorData){image=imdecode(vectorData, CV_LOAD_IMAGE_COLOR);}
    Mat get_image(){return image;}
    void start() {m_pcThread= AfxBeginThread(ThreadProc, this );}
}

int main() {
    A* a = new A();
    a->start();
    while(1) {
        Mat image = a->get_image();
    }
    delete a;
    return 0;
}

It seems that the error come from Mat image = a->get_image(); because if I return a reference instead of a copy of the object, I don't have error anymore:

Mat* get_image(){return ℑ}

and

Mat* image = a->get_image();

I read that returning a copy of an object is more elegant in C++ than a reference. So I would like to know what is wrong.

EDIT: Visual studio breaks at a->decode(vect) but it happens only when I return an object and not a reference.

EDIT 2: I edited the code to reflect the full program. I think the problem comes from the shared object a which is copy and modified at the same time. I will see if the problem still occur using a mutex.

Answer 1

The copy constructor of cv::Mat does not create a deep copy of the image. It just creates a reference to the original Mat and increases its reference count. In the following function of the class, the return statement calls the copy constructor, returning a reference to the original image, which is the probable cause of heap corruption:

Mat get_image(){ return image; }

You should return a deep copy of the image as follows, so that the original image does not get modified accidentally.

Mat get_image(){ return image.clone(); }

Answer 2

You use a without initialising it.

int main() {
    A* a;
    vector<uchar> vect;
    while(1) {
        // get the vector of data
        a->decode(vect);

Welcome to undefined behaviour, population: you.

Initialise a for a better result.

Answer 3

Then that is the thread sync problem, as you yourself suggested. The image is being constantly filled in in that infinite while loop. And in the middle of it somewhere you try to make a copy of it. Recipe for disaster. You need to take a write lock inside that loop in DoThreadProc, in each iteration. And then you need to take a read lock inside your get_image. You need to use a read/write lock library that does not starve out readers.

Alternatively you could work with mutexes/critical sections. Both that write loop, and that read (get_image) just get exclusive access to image while they are doing their work.

I am curious though - your thread procedure is decoding things in an infinite loop. What are you trying to do in there? And while reading what image do you expect? Any image at that point of time in loop iteration?