Ownership of my new Unique_ptrs?

Question

As per suggestion at a job interview I had recently, I was advised to research into the unique_ptr functionality of C++11, as a means of automated garbage collection. So I'm using an older project and replacing my raw pointers to objects created with the 'new' keyword, with unique_ptrs. However I think I have arrived at an issue of ownership.

In my mainclass.cpp (posted below) please turn your attention to the init function and the 3 unique_ptrs to new-instantiated objects I have created. Named "bg","bg2", and "theGrid". (The commented out declarations below them are how they used to be done, and switching back to this method, the program runs just fine.)

However, using the unique_ptrs, the line in the function void display():

theGrid->doGridCalculations();//MODEL

generates an access violation. This is also the first time in the sequence that any of the pointed objects are dereferenced, which leads me to believe that ownership of the unique_ptr is already lost somewhere. However, the unique_ptrs themselves are never passed into another function or container, and remain in the scope of the mainclass.cpp and therefore I've seen no opportunity to use std::move(theGrid) in order to transfer ownership to where it needs to be.

Mainclass.cpp:

#include <stdio.h>
#include <GL/glut.h> 
#include <math.h>
#include "Block.h"
#include "dStructs.h"
#include "Grid.h"
#include "Texture.h"
#include "freetype.h"
#include <Windows.h>


//////////////////////////////////////////////////////
///Declare a couple of textures - for the background
//////////////////////////////////////////
Texture* bg;
Texture* bg2;
//and theGrid
Grid* theGrid;

/////////////////////////////////////////////////
///Declare our font
/////////////////////////////////////////////////
freetype::font_data scoreFont;
/////////////////////////////////////////////////////////
//Initialize the variables
///////////////////////////////////////////////////////
typedef dStructs::point point;
const int XSize = 755, YSize = 600;


point offset = {333,145};
point mousePos = {0,0};


void init(void)
{
    //printf("\n......Hello Guy. \n....\nInitilising");
    glMatrixMode(GL_PROJECTION);    
    glLoadIdentity();
    gluOrtho2D(0,XSize,0,YSize);

    //////////////////////////
    //initialise the fonts
    /////////////////////////


    try{
    scoreFont.init("Visitor TT2 BRK Regular.ttf", 20);
    } catch (std::exception &e) {
        MessageBox(NULL, e.what(), "EXCEPTION CAUGHT", MB_OK | MB_ICONINFORMATION);

    }
    ///////////////////////////////////////////////////////////////
    ///bg new MEMORY MANAGED EDITION
    //////////////////////////////////////////////////////////////////
    unique_ptr<Texture> bg(new Texture(1024,1024,"BackGround.png"));
    unique_ptr<Texture> bg2(new Texture(1024,1024,"BackGround2.png"));
    unique_ptr<Grid> theGrid(new Grid(offset));
    /////////////////////////////////////////////////
    /// Old bad-memory-management style of pointed objects
    /////////////////////////////////////////////////
    //bg = new Texture(1024,1024,"BackGround.png");
    //bg2 = new Texture(1024,1024,"BackGround2.png");
    //theGrid = new Grid(offset);

    glClearColor(0,0.4,0.7,1);

    glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);//activate the alpha blending functionality
    glEnable(GL_BLEND);
    glLineWidth(2);         // Width of the drawing line
    glMatrixMode(GL_MODELVIEW); 
    glDisable(GL_DEPTH_TEST);
    //printf("\nInitialisation Complete");

}

void myPassiveMouse(int x, int y)
{
    //Stupid OGL coordinate system
    y = YSize - y;
    mousePos.x = x;
    mousePos.y = y;
    printf("\nthe mouse coordinates are (%f,%f)",mousePos.x, mousePos.y);
}

void displayGameplayHUD()
{
    ///////////////////////////////
    //SCORE
    //////////////////////////////
    glColor4f(0.7f,0.0f,0.0f,7.0f);//set the colour of the text
    freetype::print(scoreFont, 100,400,"SCORE: ");
    glColor4f(1.0f,1.0f,1.0f,1.0f);//Default texture colour. Makes text white, and all other texture's as theyre meant to be.

}

//////////////////////////////////////////////////////
void display()
{
    ////printf("\nBeginning Display");
    glClear(GL_COLOR_BUFFER_BIT);//clear the colour buffer

    glPushMatrix();
    theGrid->doGridCalculations();//MODEL

    point bgLoc = {XSize/2,YSize/2};
    point bgSize = {XSize,YSize};
    bg2->draw(bgLoc,bgSize);
    theGrid->drawGrid();//DISPLAY
    bg->draw(bgLoc,bgSize);

    if(theGrid->gridState == Grid::STATIC)
    {
        theGrid->hoverOverBlocks(mousePos);//CONTROLLER
    }

    displayGameplayHUD();

    glPopMatrix();

    glFlush();  // Finish the drawing
    glutSwapBuffers();
    ////printf("\nFresh Display Loaded");

    glutPostRedisplay();
}

int main(int argc, char** argv) 
{
  glutInit(&argc, argv);    // GLUT Initialization 
  glutInitDisplayMode(GLUT_RGBA|GLUT_DOUBLE); // Initializing the Display mode
  glutInitWindowSize(755,600);  // Define the window size
  glutCreateWindow("Gem Miners");   // Create the window, with caption.
  init();   // All OpenGL initialization


  //-- Callback functions ---------------------
  glutDisplayFunc(display);
  //glutKeyboardFunc(mykey);
  //glutSpecialFunc(processSpecialKeys);
  //glutSpecialUpFunc(processSpecialUpKeys);
  glutMouseFunc(mymouse);

  glutPassiveMotionFunc(myPassiveMouse);

  glutMainLoop();   // Loop waiting for event 
}

I think the ownership needs to be transferred at some point, but I don't know where.

Thanks in advance, Guy

Answer 1

These are global raw pointers:

Texture* bg;
Texture* bg2;
//and theGrid
Grid* theGrid;

These are completely unrelated unique_ptrs, local to the init function.

unique_ptr<Texture> bg(new Texture(1024,1024,"BackGround.png"));
unique_ptr<Texture> bg2(new Texture(1024,1024,"BackGround2.png"));
unique_ptr<Grid> theGrid(new Grid(offset));

When the unique_ptrs go out of scope, they are destroyed. The objects that they point to are also destroyed, because that is what unique_ptr does in its destructor. At no point in that process were the global raw pointers involved with the debacle. They were hidden by the local unique_ptrs of the same name.

You should change your global raw pointers to unique_ptrs. Then you can set them (don't re-declare them) in the init function like this:

bg.reset(new Texture(1024,1024,"BackGround.png"));
bg2.reset(new Texture(1024,1024,"BackGround2.png"));
theGrid.reset(new Grid(offset));

Answer 2

The unique pointers that you create in your init function do not modify the pointers declared at file scope, the ones at file scope are default-initialised to 0 or nullptr (I'm not that well versed at C++11 so I'm not sure which).

What you're doing in the init function is creating three new objects with names that shadow the ones at file scope, so when you go to use the ones at file scope you get an access violation because they are never set to point at anything valid.

Answer 3

Your unique_ptr<Grid> in init is local to that function. The unique_ptr<Grid> will go out of scope at the end of the function, destroying itself and the Grid it owns. It seems like you want to actually have a global object unique_ptr<Grid> theGrid; which replaces the Grid* theGrid; you've got at the momement. Then in init you can do:

theGrid.reset(new Grid(offset));

The theGrid that is being accessed in display is the global theGrid of type Grid*.

The exact same is true for the other unique_ptrs you've tried to create.

Of course, rather than a global object, it would be much better to be passing these objects around, but your use of GLUT makes that a bit painful.