Question

Current Implementation

I have a class containing unique_ptr fields which depend on one other:

class ResourceManager {
  ResourceManager() {}

  ResourceManager(A* a_ptr) :
    b_ptr(new B(a)),
    c_ptr(new C(b_ptr.get())) {}

  ResourceManager& operator=(ResourceManager&& that) {
    // Call destructor, then construct a new instance on top
    ~ResourceManager();
    ResourceManager* new_this = new(this) ResourceManager();

    // Surely this must be the case, right?
    // Is there any reason to prefer using either?
    assert(new_this == this);

    new_this->b_ptr = that.b_ptr;
    new_this->c_ptr = that.c_ptr;

    return *new_this;
  }

  unique_ptr<B> b;
  unique_ptr<C> c;
};

Use case

The use case here is that I would like to reassign new values to the pointers, whilst keeping the ResourceManager as a stack-allocated variable, or as a non-pointer class member.

With my current setup I imagine using it something like this:

A a, another_a;
ResourceManager r(&a);

// Use r...

// Destroy old ResourceManager and create the new one in place.
r = ResourceManager(&another_a);

The reason this is even a problem is due to the fact that B and C are non-assignable (for e.g. file streams)

Ugly Alternative

An alternative uglier (and dangerous) method would be to explicitly reset the unique_ptr fields crucially in reverse order (remember that C depends on B, and hence must be destructed first), effectively mimicking the default destruction behaviour.

ResourceManager& operator=(ResourceManager&& that) {
  // Mimic destructor call (reverse-order destruction)
  c_ptr.reset();
  b_ptr.reset();

  b_ptr = that.b_ptr;
  c_ptr = that.c_ptr;

  return *this;    
}

Note that a wrong implementation would be to simply use the default assignment operator for ResourceManager. This will assign the field in-order which implies in-order destruction of the unique_ptrs, whereas we require reverse-order destruction.

Questions

Is this usage of this pointer with placement new and the explicit destructor call safe?

Must I use the returned new_this pointer as opposed to the original this pointer (for example, if the this pointer technically becomes invalidated after calling the destructor)?

Are there any better suggested ways to achieve this? If add more such unique_ptr fields to the class, I would have to make sure that I add a copy to the assignment operator. For instance, is it possible to call the move constructor instead, like so:

ResourceManager& operator=(ResourceManager&& that) {
  // Call destructor
  ~ResourceManager();

  // Move-construct a new instance on top
  ResourceManager* new_this = new(this) ResourceManager(that);
  return *new_this;
}
Was it helpful?

Solution

Your solution seems overly complex.

I would code it like this:

class ResourceManager {
  ResourceManager() {}

  ResourceManager(A* a_ptr) :
    b_ptr(new B(a)),
    c_ptr(new C(b_ptr.get())) {}

  ResourceManager& operator=(ResourceManager&& that) 
  {
    // the order of these moves/assignments is important
    // The old value of *(this->c_ptr) will be destroyed before
    // the old value of *(this->b_ptr) which is good because *c_ptr presumably
    // has an unprotected pointer to *b_ptr.
    c_ptr = std::move(that.c_ptr);
    b_ptr = std::move(that.b_ptr);
    //  (a better solution might be to use shared_ptr<B> rather than unique_ptr<B> 
    return *this;
  }

  unique_ptr<B> b_ptr;
  unique_ptr<C> c_ptr;
};

Note: When the move assignment returns, that will "empty" meaning both that.b_ptr and that.c_ptr are nullptr. This is the expected result of a move assignment.

Or if "reconstructing" the target of the assignment is important (assuming there's extra code not shown in this example that makes it so) I might add a move constructor and a swap method like so:

 ResourceManager(ResourceManager&& that)
 : b_ptr(std::move(that.b_ptr)),
   c_ptr(std::move(that.c_ptr))    
 {
 }

 void swap(ResourceManager & that)
 {
   b_ptr.swap(that.b_ptr);
   c_ptr.swap(that.c_ptr);  
 }

 ResourceManager& operator=(ResourceManager&& that) 
 {
     ResourceManager temp(std::move(that));
     this->swap(temp);
     return *this;
 }  
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top