Smart pointer as member

[shadowmouse]

I'm currently making a class that has an optional member (a texture). As textures are very big, I'd be using references or pointers anyway but because it is optional I'm thinking that I'll need a pointer. But this always causes a dilemma. I don't want to use a unique or shared pointer because it's entirely possible that the passed texture was created on the stack and wouldn't this lead to either deleting something that's on the stack or calling the destructor twice (when the object goes out of scope and when the pointer goes out of scope)? Is the solution just to use a normal pointer and hope it isn't assigned a stack variable? I've looked up weak_ptr and I'm not sure if that's what I want, especially as the reference on this site isn't finished.

[MiiNiPaa]

Does your class own that texture? I.e. should it delete when your class is destroyed or it is someone else responsibility?

If you own it, unique_ptr is the way to go. To avoid stack created textures, do not create them on stack. Prohibit that if needed. (It might make sense to use shared pointer to texture if several object uses same texture)

If you don't, raw pointer is enough. But you need to make sure that someone properly cleans it up and your texture outlives your class (to avoid dangling references)

[shadowmouse]

If I had to choose, I'd go with the class not owning the texture, but is there any way to have a smart pointer that only deletes heap allocated objects?

[MiiNiPaa]

There is no easy and portable way to determine if arbitrary object was allocated on stack or heap. Also stack object are very short loved. you need to make sure that your object is destroyed before texture goes out of scope.

[Duthomhas]

But you can supply your std::shared_ptr with a deleter.

#include <memory>

template <typename T>
struct OptionalArrayDeleter
{
  bool is_delete;
  OptionalArrayDeleter( bool is_delete = true ): is_delete( is_delete ) { }
  void operator () ( T* p )
  {
    if (is_delete) delete [] p;
  }
};

template <typename T>
std::shared_ptr <T> MakeOptionalArrayDeleter( T* p, bool is_delete = true )
{
  return std::shared_ptr <T> ( p, OptionalArrayDeleter <T> ( is_delete ) );
}

#include <iostream>

int main()
{
  int xs[ 5 ] = { 2, 3, 5, 7, 11 };
  std::shared_ptr <int> hp = MakeOptionalArrayDeleter( new int[5] );
  std::shared_ptr <int> sp = MakeOptionalArrayDeleter( xs, false );
  
  for (unsigned n = 0; n < 5; n++)
    hp.get()[ n ] = sp.get()[ n ];
  
  for (auto x = hp.get(); x < hp.get() + 5; x++)
    std::cout << *x << " ";
  std::cout << "\n";
}

Hope this helps.

[Gamer2015]

this may be a little bit off-topic but you could use a modified form of Herb Sutters favourite 10-liner to load textures, then there would be nothing like double-loading a texture, you'd have all objects on the heap and it's also thread-save.

I hope I didn't miss anything here, note that this code uses some c++11 features

#include <map>
#include <mutex>
#include <memory>

std::shared_ptr<Texture> get_texture(const std::string& path) {
    static std::map<std::string, std::weak_ptr<Texture> > cache;
    static std::mutex mu;
    
    std::lock_guard<std::mutex> lock(mu);
    auto& item = cache[path];
    auto sp = item.lock();
    if(sp == nullptr) 
        item = sp = load_texture(path); // some function to load your texture
    
    return sp;    
}

[MiiNiPaa]

Sutters 10-liner has some problems which steams from map storing weak references to the dead data, keeping refcount block alive, and do not cleans it up.
If Texture stores everything inside itself (without using dynamic memory) and load_texture uses make_shared(), then you would be better just loading everything in memory and keeping it here.
You will need to make sure that either Texture itself is small, or load_texture does not uses make_shared's single block allocation optimisation.

[JLBorges]

A simple kludge that solves the problem at hand in a natural manner (references for named objects and pointers for objects with dynamic storage duration). Nonetheless, it is still a kludge.

#include <iostream>
#include <memory>

struct texture { /* ... */ virtual ~texture() { std::cout << "destrutor\n" ; } };

struct A
{
    A() = default ;

    // pass texture objects which do not have a dynamic storage duration by reference
    A( texture& t ) : ptr( std::addressof(t), []( texture*){} ) {} // do nothing deleter

    // pass texture objects with dynamic storage duration wrapped in a shared pointer
    template < typename TEXTURE_TYPE >
    A( const std::shared_ptr<TEXTURE_TYPE>& pt ) : ptr(pt) {}

    template < typename TEXTURE_TYPE >
    A( std::shared_ptr<TEXTURE_TYPE>&& pt ) : ptr( std::move(pt) ) {}

    std::shared_ptr<texture> ptr ;
};

int main()
{
    static texture x ; // texture#1
    A a1(x) ; // passed by reference 

    A a2( std::make_shared<texture>() ) ; // texture#2

    auto y = std::make_shared<texture>() ; // texture#3
    A a3(y) ;

    struct derived_texture : texture { ~derived_texture() { std::cout << "derived class "; } };

    derived_texture d1 ; // texture#4
    A a4(d1) ; // passed by reference. note: a4 is destroyed before d1

    A a5( std::make_shared<derived_texture>() ) ; // texture#5
}

http://coliru.stacked-crooked.com/a/87c5274793c1ce5d

[Gamer2015]

If Texture stores everything inside itself (without using dynamic memory) and load_texture uses make_shared(), then you would be better just loading everything in memory and keeping it here.

Which would be no problem.
You could just modify it to hold shared_ptrs instead of weak_ptrs, which is the same as loading everything in memory and keeping it there.

Sutters 10-liner has some problems which steams from map storing weak references to the dead data, keeping refcount block alive, and do not cleans it up.

Yeah, that's true.
You could make a seperate *.cpp file, put cache in an anonymous namespace and put a clean function somewhere, can't come up with anything else at this time of the day.

[dhayden]

I'm currently making a class that has an optional member (a texture). As textures are very big, I'd be using references or pointers

This seems to imply that if the a texture was smaller, you'd make it a member of the class. That means it wouldn't be shared among members. That means shared_ptr<> is out. And assuming that instance1 = instance2 should not cause it to disappear from instance2, that means unique_ptr is out too (unless you create an assignment operator to copy the object).

Since you'll have to add some code anyway, I'd make it a private raw pointer. When assigning a texture to it, you'll have to define whether the class takes ownership of the texture (implying that it must be allocated from the heap), or makes a copy (which might be time consuming). Neither way is better than the other, the important thing is that you must decide what should happen, document it in the header file, and then honor that design.

[MiiNiPaa]

You could just modify it to hold shared_ptrs instead of weak_ptrs, which is the same as loading everything in memory and keeping it there.

Exactly my point. Having a cache with in on-demand loading is unnesesary in this case.

Implementation you posted should be used very carefully. You can fix it, but it won't be "10-liner" anymore.