Weak pointers to vector elements idea

Question

I want to store some objects in linear memory, but sometimes I want to use these objects in some place for some time, but in the meantime object can be deleted, so I want to use weak_ptr's. This is my idea how to handle this, what you think ? Class which have vector of my object.

world3d.h

class World3d
{
public:
 void Render();
 std::weak_ptr<Object3d> CreateObject3d();
private:
 std::vector<Object3d> m_objects;
};

world3d.cpp

void World3d::Render()
{
}
std::weak_ptr<Object3d> World3d::CreateObject3d()
{
 m_objects.emplace_back();
 return m_objects.back().GetWPtr();
}
void World3d::SetCamera(std::shared_ptr<Camera> pCamera)
{
 m_pCamera = pCamera;
}

object3d.h

class Object3d
{
public:
 Object3d();
 virtual ~Object3d();
 std::weak_ptr<Object3d> GetWPtr();
private:
 std::shared_ptr<Object3d> thisPtr;
 Matrix m_xform;
};

object3d.cpp

void null_deleter(Object3d *) {}
Object3d::Object3d()
{
 thisPtr.reset(this, &null_deleter);
}
Object3d::~Object3d()
{
}
std::weak_ptr<Object3d> Object3d::GetWPtr()
{
 return thisPtr;
}

• Toby Speight: Why the intrusion into Object3D? Is there a good reason not to simply use a std::vector<std::shared_ptr<Object3d>>?
• piotrek: @TobySpeight I Want to have large number of these elements, like 1000 - 10000, so I want to have them in contiguous memory.
• Incomputable: @piotrek, the right solution would be creating allocator, then passing deleter into shared_ptr to deallocate it at the end.

Answer 1

Very suspicious approach

Lifetime of elements in std::vector is manager by the vector. I doubt that std::shared_ptr can intervene here at all. The idea is both hard to understand and hard to prove its correctness. Though, even if std::shared_ptr would be able to intervene...

Reference invalidation

Imagine the container already gave out some pointers, then someone puts more elements into the container and it reallocates elements into new location. The handed out pointers will not be able to get lock()ed, and the whole thing will fall. Even if nobody puts elements into it, problem description mentions deletion. On deletion pointers will get invalidated too.

Solution

I see two ways of solving this problem:

1. Use std::list. The problem requires references that are not invalidated in case of deletion. If you're not traversing the list a lot, and doing some heavy computation on it, probably cost of segmented memory layout will be negligible/affordable.

2. Block the thread that does deletion/resizing of the vector when you're performing some operations on already existing ones. This is the hard way, but arguably can be win-win situation if thread synchronization is done correctly.

Code Review

• Initialize members on construction where possible

  It is possible to pass this, &null_deleter to thisPtr in member initialization list:

  Object3d::Object3d():
   thisPtr(this, &null_deleter)
  {}
  

• Redundant destructor.

  If it is empty and is not virtual, just don't declare/define it. If it is virtual, you can just = default it.

• Double copy.

  void World3d::SetCamera(std::shared_ptr<Camera> pCamera)
  {
   m_pCamera = pCamera;
  }
  

  Copies pCamera twice. Pass by const reference to avoid second copy. May be compiler will figure it out, but in more complex case it might not be able to.

Testing

Testing is important.

• Build script with testing.

  There should be some script (CMake, make, python, whatever is available) to build the project and test it. It will provide confidence in claims.

• It is better in the long run

  Tests are like an investment. The time spend will come back when changes are made to the code.

Conclusion

Usually code has some places where it is slower, and some places that are optimized well. You could trade performance of some components for the other, or for easier implementation and correctness.