CODEWITHSUNDEEP
c++memory-managementshared-ptropengl-2.0
Ramy Al Zuhouri
Ramy Al Zuhouri

Reputation: 21996

Class design: how to return a shared_ptr: reference or copy

This is the scenario: I have a class named Program, which holds three shared_ptr: to the vertex, geometry and fragment shader. When a Shader object gets constructed, it creates the shader with glCreateShader, and it also compiles it.

The Shader denstructor automatically calls glDeleteShader. So the problem is that if I do the following:

  1. Create a shader object;
  2. Copy it;
  3. Destroy the copy.

Also the original copy gets invalidated, because when the copy is destroyed it calls glDeleteShader. That's a design problem, I believe.

So I avoided this problem by just using pointers. Now the Program class holds the shaders. I created a method that returns the shared_ptr to the vertex, geometry and fragment Shader objects. My doubt is: should I return the shared_ptr like this: 

const shared_ptr<Shader>& getVertex_shader_ptr() const
{
    return vertex_shader_ptr;
}

Or like this: 

shared_ptr<Shader> getVertex_shader_ptr() const
{
    return vertex_shader_ptr;
}

My fear is that the problem that I described above occurs again: the shared_ptr gets deallocated and it invalidates the OpenGL shader.

Upvotes: 1

Views: 6008

Answers (4)

hcc23
hcc23

Reputation: 1280

Without knowing much about the underlying shared pointer issues, I found this post by Thiago Macieira (presumably a Qt/Trolltech employee) fairly helpful in getting a better idea about whether to return a value or a const&: http://lists.trolltech.com/qt-interest/2007-11/thread00209-0.html

Quoting from that post, here is the core argument for returning a value from a const function:

The reason why it's:

T at(const Key& k) const;

instead of:

const T &at(const Key &k) const;

is because it's like that everywhere in Qt. We do not return refs-to-const anywhere. In order to return a ref-to-const, we require a variable to exist somewhere, which mandates the internal structure of the class. By returning a value, we can do anything we want internally.

I am not certain though, if that is applicable to your issue... (Still learning myself ;) )

Upvotes: 1

Bill
Bill

Reputation: 14695

Unless it's valid for your shader to be NULL, you should just return a reference to it:

const Shader& getVertex_shader() const
{
    return vertex_shader;
}

If it's valid for your shader to be NULL, but only your program is responsible for deleting it, just return a pointer:

const Shader* getVertex_shader_ptr() const ...

The semantics of a shared pointer is for when there's no clear ownership of the shader. If that's the case, then return by value. The semantics are that two pieces of the program have an interest in the object not being cleaned up, and they both need a way to keep it alive.

shared_ptr<Shader> getVertex_shader_ptr() const ...

Upvotes: 5

Piotr99
Piotr99

Reputation: 531

I would prefer to return by value. If you return by reference, you might run into the chance of a dangling reference to the shared_ptr, if at some point the instance is destroyed and some variable still holds a reference to the shared_ptr.

This situation is exactly what the smart pointers were supposed to avoid, but their reference counting only works safely if you avoid return them by copying. This would invalidate the entire point of using a shared_ptr here.

Upvotes: 2

Ben Hymers
Ben Hymers

Reputation: 26556

Either way is fine here.

It's a member so it's safe to return a reference to it, as long as the caller doesn't hold it by reference then destroy the Program.

If you'd returned by non-const reference then the caller could call reset on your pointer which would call glDeleteShader if no other copies of the pointer exist. As it is though, it'll behave as you want - the Shader will only be destroyed when no more references to it exist.

I'd personally return the shared_ptr by value, but that's just personal preference.

EDIT: Assuming you meant you're worried about correctness when you copy a Program (not a Shader), don't worry about that either; the two Programs will have shared_ptrs to the same Shader which won't be destroyed until both Programs are. This may or may not be what you want (you might want to allocate a completely separate Shader on copy) but it's safe.

Upvotes: 2

Related Questions