c++ shared_ptr of stack object

Question

I have been learning managed pointers lately and ran into the following scenario.

I am implementing a model/controller class for a game view. My view, will render things in the model. Pretty straight forward. In my main function, I instantiate all three like this:

RenderModel m;
m.AddItem(rect); // rect gets added just fine, it's an "entity" derivee
RenderView v;
v.SetModel(m);

My render view class is pretty straightforward:

class RenderView
{
public:
explicit RenderView();
~RenderView();

void Update();

void SetModel(RenderModel& model);

private:
// disable
RenderView(const RenderView& other);
RenderView& operator=(const RenderView& other);

// private members
boost::scoped_ptr<RenderModel> _model;
};

The implementation for setView is pretty standard:

void RenderView::SetModel(RenderModel& model)
{
    _model.reset(&model);
}

The key to this is, the view stores a model in a smart pointer. However in main, the model was allocated on the stack. When the program exits, the memory gets deleted twice. This makes sense. My current understanding tells me that anything which gets stored in a smart_ptr (of any kind) should not have been allocated on the stack.

After all the above setup, my question is simple: how do I dictate that a parameter was not allocated on the stack? Is accepting a smart pointer as a parameter the only solution? Even then I could not ensure that someone using my view class could not do something incorrect such as:

// If I implemented SetModel this way:
void RenderView::SetModel(const std::shared_ptr<RenderModel>& model)
{
    _model.reset(&*model);
}

RenderModel m;
RenderView v;
std::shared_ptr<RenderModel> ptr(&m); // create a shared_ptr from a stack-object.
v.SetModel(ptr);

Answer 1

You should probably define the semantics of your class more clearly. If you want RenderView to be the owner of the RenderModel it should create it on its own (maybe get in the constructor some identifier to use with a factory).

I've seen classes that receive ownership of objects, and it was defined explicitly that this objects must be on the heap, but this is, to my opinion, error prone, just like the error you now encountered. You can not give a stack object to a smart pointer that expects it to be on the heap (because it will use delete on it when it wants to clean it).

Answer 2

In short: define custom deleter. In case of a smart pointer on a stack object, you can construct the smart pointer with a custom deleter, in this case a "Null Deleter" (or "StackObjectDeleter")

class StackObjectDeleter {
public:
    void operator () (void*) const {}
};

std::shared_ptr<RenderModel> ptr(&m, StackObjectDeleter());

The StackObjectDeleter replaces the default_delete as the deleter object. default_delete simply calls delete (or delete []). In case of StackObjectDeleter, nothing will happen.

Answer 3

You should require the user to properly pass input. First change your input type to a smart pointer (rather than a reference variable). Second have them properly pass the smart pointer with a that doesn't do anything (NoDelete below, example).

A hackish method would be to check the memory segment. The stack is always going to grow down from kernel space (I think 0xC0000000 in 32 bit, something like 0x7fff2507e800 in 64 bit, based on the code below). So you could guess based on memory location whether it is a stack variable or not. People will tell you it isn't portable, but it kind of is, unless you are going to have stuff deployed on embedded systems.

#include <iostream>
#include <memory>

using namespace std;

class foo
{
    public:
    foo(shared_ptr<int> in) {
        cerr << in.get() << endl;
        cerr << var.use_count() << endl;
        var = in;
        cerr << var.use_count() << endl;
    };

    shared_ptr<int> var;
};

struct NoDelete {
    void operator()(int* p) {
        std::cout << "Not Deleting\n";
    };
};

int main()
{
    int myval = 5;

    shared_ptr<int> valptr(&myval, NoDelete());
    foo staticinst(valptr);

    shared_ptr<int> dynptr(new int);
    *dynptr = 5;
    foo dynamicinst(dynptr);
}

Answer 4

class ModelBase
{
    //.....
};

class RenderView
{
    //..........
public:
    template<typename ModelType>
    shared_ptr<ModelType> CreateModel() {
        ModelType* tmp=new ModelType();
        _model.reset(tmp);
        return shared_ptr<ModelType>(_model,tmp);
    }

    shared_ptr<ModelBase> _model;
    //......
};

If the model class constructor has parameters, it's possible to add parameters to method CreateModel() and use C++11 perfect forwarding technics.

Answer 5

You should go back and think on the design. The first code smell is that you are taking an object by reference and then trying to manage it as a smart pointer. That is wrong.

You should start by deciding who is responsible for the resource, and design around it, if RenderView is responsible for the management of the resource, then it should not accept a reference, but rather a (smart) pointer. If it is the sole owner, the signature should take a std::unique_ptr (or std::auto_ptr if your compiler+libs don't support unique_ptr), if ownership is diluted (prefer to make a sole owner whenever possible), then use a shared_ptr.

But there are also other scenarios where RenderView does not need to manage the resource at all, in which case it can take the model by reference and store it by reference if it cannot be changed during the lifetime of RenderView. In this scenario, where RenderView is not responsible for management of the resource, it should not try to delete it ever (including through a smart pointer).

Answer 6

The way you described what you want to do is completely wrong. In the MVP design pattern, the view should not access the model directly, but should send commands to the presenter (by calling presenter's functions).

Anyway, other's have answered your question : your model object has to be allocated on the heap, like this :

std::shared_ptr<RenderModel> ptr( new RenderModel );
RenderView v;
v.SetModel(ptr);

otherwise your shared_ptr object is going to try to delete a stack object.

Answer 7

how do I dictate that a parameter was not allocated on the stack?

Yes, require the caller to provide a std::shared_ptr<RenderModel>. If the caller misconstructs the std::shared_ptr, that's the caller's problem, not yours.

If you intend for a RenderView to be the sole owner of a particular RenderModel, consider having the function take a std::unique_ptr or std::auto_ptr instead; this way it is clear that the caller should not retain ownership of the object after it calls the function.

Alternatively, if RenderModel is cheap to copy, make a copy of it and use the copy:

_model.reset(new RenderModel(model));