Replacing raw pointers in vectors with std::shared_ptr

Question

I have the following structure:

typedef Memory_managed_data_structure T_MYDATA;
std::vector<T_MYDATA *> object_container;
std::vector<T_MYDATA *> multiple_selection;
T_MYDATA * simple_selection;

Edit: this may be very important: the Memory_managed_data_structure contains, among other things, a bitter, raw pointer to some other data.

It aims to be a very simple representation of an original container of memory managed objects (object_container) and then a "multiple_selection" array (for selecting many objects in the range and doing various operations with them) and a "simple_selection" pointer (for doing these operations on a single object).

The lifetime of all objects is managed by the object_container while multiple_selection and simple_selection just point to some of them. multiple_selection and simple_selection can be nullified as needed and only object_container objects can be deleted.

The system works just fine but I am trying to get into shared_ptrs right now and would like to change the structure to something like:

typedef Memory_managed_data_structure T_MYDATA;
std::vector<std::shared_ptr<T_MYDATA> > object_container;
std::vector<std::shared_ptr<T_MYDATA> > multiple_selection;
std::shared_ptr<T_MYDATA> simple_selection;

Again, the object container would be the "owner" and the rest would just point to them. My question is, would this scheme wreak havok in the application?. Is there something I should know before snowballing into these changes?. Are not shared_ptr the appropriate kind of pointer here?.

I can somewhat guarantee that no object would exists in multiple_selection or simple_selection if it is not in object_container first. Of course, no delete is ever called in multiple_selection or simple_selection.

Thanks for your time.

Edit: Forgot to mention, never used any of these automated pointers before so I may be wildly confused about their uses. Any tips and rules of thumb will be greatly appreciated.

Answer 1

You say, that the object container would be the "owner" of the objects in question. In that case, that you have a clear owning relationship, using std::shared_ptr is not ideal. Rather, stick with what you have.

However, if you cannot guarantee, that a pointer has been removed from multiple_selection and/or simple_selection before it is deleted, you have to act. One possible action could be, that you use shared_ptr. In that case, an object could still continue to exist in one of the selections, even, if it is removed (via shared_ptr::reset or just assigning a null value) from object_container.

The other alternative is to make sure, that objects get removed thoroughly: If something is to be deleted, remove ALL references to it from the selections and from the object_container, and THEN delete it. If you strictly follow this scheme, you don't need the overhead of shared_ptr.

Answer 2

I can somewhat guarantee that no object would exists in multiple_selection or simple_selection if it is not in object_container first.

If you 150% sure, than there is no need for smart ptr.

Reason you may need it in this situation is debug, I think. In case you describe - multiple_selection and simple_selection is not shared_ptr, but weak_ptr.

Code with error:

  std::vector<int*> owner_vector;
  std::vector<int*> weak_vector;

  int* a = new int(3);

  owner_vector.push_back(a);
  weak_vector.push_back(a);

  std::for_each(
      owner_vector.begin(),
      owner_vector.end(),
      [](int* ptr) {
        delete ptr;
      }
  );

  std::for_each(
      weak_vector.begin(),
      weak_vector.end(),
      [](int* ptr) {
        *ptr = 3; // oops... usage of deleted pointer
      }
  );

You can catch it with smart pointers:

  std::vector<std::shared_ptr<int>> owner_vector;
  std::vector<std::weak_ptr<int>> weak_vector;

  {
    auto a = std::make_shared<int>();

    owner_vector.push_back(a);
    weak_vector.push_back(a);
  }

  std::for_each(
      owner_vector.begin(),
      owner_vector.end(),
      [](std::shared_ptr<int>& ptr) {
        ptr.reset(); // memory delete
      }
  );

  std::for_each(
      weak_vector.begin(),
      weak_vector.end(),
      [](std::weak_ptr<int>& ptr) {
        assert(!ptr.expired()); // guarantee to be alive
        auto shared_ptr = ptr.lock();
        *shared_ptr = 3;
      }
  );

In last example you will have assert failed, but not undefined/segmentation fault. In not debug case you can disable shared_ptr overhead.