Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it safe to use placement new on 'this' pointer

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;
}
like image 312
WaelJ Avatar asked Apr 29 '14 16:04

WaelJ


People also ask

What is placement new and why would I use it?

Placement new is a variation new operator in C++. Normal new operator does two things : (1) Allocates memory (2) Constructs an object in allocated memory. Placement new allows us to separate above two things. In placement new, we can pass a preallocated memory and construct an object in the passed memory.

In which cases we would need to use placement new?

Use cases. Placement new is used when you do not want operator new to allocate memory (you have pre-allocated it and you want to place the object there), but you do want the object to be constructed.

Does placement New allocate memory?

Because placement new does not allocate memory, you should not use delete to deallocate objects created with the placement syntax. You can only delete the entire memory pool ( delete whole ). In the example, you can keep the memory buffer but destroy the object stored in it by explicitly calling a destructor.

Does vector use placement new?

vector::emplace_back Appends a new element to the end of the container. The element is constructed through std::allocator_traits::construct, which typically uses placement-new to construct the element in-place at the location provided by the container.


1 Answers

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;
 }  
like image 160
Dale Wilson Avatar answered Oct 24 '22 08:10

Dale Wilson