Creating a new object destroys an older object with different name in C++

Question

First question here!

So, I am having some problems with pointers in Visual C++ 2008. I'm writing a program which will control six cameras and do some processing on them so to clean things up I have created a Camera Manager class. This class handles all operations which will be carried out on all the cameras. Below this is a Camera class which interacts with each individual camera driver and does some basic image processing.

Now, the idea is that when the manager is initialised it creates two cameras and adds them to a vector so that I can access them later. The catch here is that when I create the second camera (camera2) the first camera's destructor is called for some reason, which then disconnects the camera.

Normally I'd assume that the problem is somewhere in the Camera class, but in this case everything works perfectly as long as I don't create the camera2 object.

What's gone wrong?

CameraManager.h:

#include "stdafx.h"

#include <vector>
#include "Camera.h"

class CameraManager{

    std::vector<Camera>     cameras;

public:

    CameraManager();
    ~CameraManager();

    void CaptureAll();
    void ShowAll();

};

CameraManager.cpp:

#include "stdafx.h"

#include "CameraManager.h"


CameraManager::CameraManager()
{

    printf("Camera Manager: Initializing\n");
    [...]
    Camera *camera1 = new Camera(NodeInfo,1, -44,0,0);
    cameras.push_back(*camera1);

    // Adding the following two lines causes camera1's destructor to be called. Why?
    Camera *camera2 = new Camera(NodeInfo,0,  44,0,0);
    cameras.push_back(*camera2);

    printf("Camera Manager: Ready\n");

}

Camera.h

#include "stdafx.h"

// OpenCV
#include <cv.h>
#include <highgui.h>

// cvBlob
#include "cvblob.h"

// FirePackage
#include <fgcamera.h>

using namespace cvb;

class Camera{

public:

    int cameraID;

    double x, y,z, FOVx, FOVy;

    IplImage *image, *backgroundImage, *labeledImage; 

    CvBlobs   blobs;

    Camera(FGNODEINFO NodeInfo[], int camID, float xin, float yin, float zin);

    ~Camera();

    void QueryFrame();

    void ProcessFrame();

    void GrabBackground();

    void LoadCalibration();

    void Show();

private:

    // ======= FirePackage  ======
    CFGCamera  FGCamera;
    UINT32     Result;
    FGNODEINFO MyNodeInfo;
    UINT32     NodeCnt;
    FGFRAME    Frame;

    // ======= Camera Configuration  ======
    // Trigger Settings
    UINT32  nOn, nPolarity, nSrc, nMode, nParm, BurstCount, DMAMode;

    // Image Settings
    UINT32  AutoExposure, Shutter, Gain, Brightness, Gamma;

    // Image Format Settings
    UINT32  Format, Mode, Resolution, ColorFormat, FrameRate;

    // Structures
    UINT32  TriggerValue;
    UINT32  FormatValue;
    UINT32  DFormatValue;

    // OpenCV Calibration matrices
    CvMat    *intrinsics, *distortion;  
    IplImage *mapx, *mapy;

    void SetUpFirePackage();

    void SetUpOpenCV();

};

Camera.cpp:

#include "stdafx.h"

#include "Camera.h"



Camera::Camera(FGNODEINFO NodeInfo[], int camID, float xin, float yin, float zin) 
    {

        cameraID = camID;
        x = xin;
        y = yin;
        z = zin;
        FOVx = 42.6;
        FOVy = 32.5;

        MyNodeInfo = NodeInfo[cameraID];

        SetUpFirePackage();
        SetUpOpenCV();

        // Grab the first frame
        printf("Waiting for frame...\n");
        QueryFrame();

    };

//Destructor
Camera::~Camera()
{   

        // Stop the device
        FGCamera.StopDevice();

        // Close capture
        FGCamera.CloseCapture();

        // Disconnect before ExitModule
        FGCamera.Disconnect();

        // Exit module
        FGExitModule();

        cvReleaseImage(&image);
    };
[...]
};

Answer 1

You need to get clear the difference between objects and pointers to objects.

Your CameraManager contains a vector of Camera so you must expect your cameras to be copied as the vector expands. This means that copies will be created and the old copies destroyed at certain points in the lifetime of the container.

This call pushes a copy of the parameter (the camera pointed to by camera1) into the vector.

cameras.push_back(*camera1);

When the second Camera is pushed into the vector it is not the Camera pointed to by camera1 that is destroyed but a copy of this Camera that was pushed into the vector. As a side note, you have a memory (and object) leak as camera1 points to an object that you allocated dynamically with new but which you don't delete.

It sounds as though you are not prepared for your Camera objects to be copied. It may be that you are better off with a container of pointers (or smart pointers to help with clean deallocation) or it may be possible for you to alter the way your Camera class works to cope correctly with being copied. Without seeing the Camera class it is difficult to know which is more appropriate.

Answer 2

You're storing raw pointers in your Camera class, but not defining a copy constructor or assignment operator (both of which std::vector requires for the above to work correctly). So, you get compiler-generated versions that do a shallow copy.

As a result of cameras.push_back(*camera1);, you're actually creating a temporary that is destroyed after the push_back returns. This temporary's destructor will release the resources you allocated in the statement Camera *camera1 = new Camera(NodeInfo,1, -44,0,0);, and the instance in the vector will have pointers to resources that are no longer valid.

You need to either not store the Camera by value in the vector (use a shared pointer of some sort) and explicitly mark the type as noncopyable/assignable, or you should provide the necessary copy/assignment semantics.

Also, not to mention you're leaking both camera1 and camera2.