I have some code that manages a queue of unique_ptrs. In its main loop, it pops an entry off of the queue, does something with it, and then allows the element to go out of scope and be deleted. So far, no problem.
I would like to add a feature where instead of necessarily deleting the element, we possibly move it somewhere else. The catch is that only the element itself knows whether to do this or where it needs to go.
The main loop is something like:
std::deque< std::unique_ptr<Object> > queue;
while( !queue.empty() ) {
auto element = std::move( queue.front() );
queue.pop_front();
element->do_something();
element->recycle( std::move(element) ); // ?
}
If recycle weren't a method of Object, this would be a non-issue, but because it is a method of object, I'm not sure what it should look like. With the above code, I'd have something like:
class Object {
public:
virtual void do_something();
virtual void recycle( std::unique_ptr<Object> unique_this ) {
// move unique_this somewhere, or don't and allow it to be deleted
}
};
Is this safe? If recycle doesn't move unique_this, it will delete this in one of its own methods (at the end of the method, though). Is that a problem?
A possibility I thought of to avoid the deletion-of-this would be to have recycle take a unique_ptr by lvalue reference, which I realize is kind of weird. The idea is that it would move it somewhere if it wanted to, but if it didn't, the object wouldn't be deleted until after recycle returned. It doesn't necessarily avoid the problem of possibly deleting an object in one of its methods, though, because we'd have to make sure that wherever we moved it to didn't immediately delete it.
If it's not safe to do something like this, the fallback plan would be to have Object return a recycler (i.e., a callable which isn't a method of Object that does the recycling). I think that should be possible, but managing the type and lifetime of the recycler would be kind of messy given that different objects will want to use different recyclers.
It seems to me that the first idea (passing a unqiue_ptr by value) is the best unless it violates any rules. Is that safe? Is there a better way to do this?
element->recycle(std::move(element)); is problematic before C++17:
element-> might be evaluated after the move constructor, and so be nullptr. So unsafe.
Safer then, would be to change to:
virtual void recycle( std::unique_ptr<Object>&& unique_this )
Remember than it is not the std::move which "moves", but the move-construction.
Alternative
auto ptr = element.get(); // element might become nullptr just below,
// so kept "old" non-nullptr value
ptr->recycle(std::move(element));
would be possible, but is error prone (require comment, or might be badly refactorized in wrong code)
Since C++17, evaluation order (13) specify that element-> is evaluated before the evaluation of the argument; So it is safe.