Unique_ptr and emplace_back.

Started by
5 comments, last by SiCrane 10 years, 4 months ago

I was wondering if this code is leak safe:



m_clients.emplace_back(std::unique_ptr<Client>(new Client()));

Logically I see two throw points, new can obviously throw with bad_alloc, and I -believe- the unique_ptr constructor can throw, but I'm not entirely sure about that. I'm not sure if the allocation ends up a no-op with a certain combination of setup like this. As far as I know something like just new Client() can leak, since it would involve a copy operation, but I'm not totally sure.

I can't seem to find any information on the order of operations when you pass a constructor to variadic templates like this. For instance I wasn't sure if it say.. called new and -then- passed the address to emplace_back, and I wasn't sure if emplace_back could throw at that point either. In which case it would never even get to constructing the object.

m_clients is a vector<std::unique_ptr<Client>> for those wondering.

So basically, is it leak safe? Why is/isn't it?

Advertisement

This is one of the two reasons why you should use make_unique() (which doesn't exist merely because of oversight, and will be added to C++14).

Define it for yourself like:


//std::make_unique implementation (it was forgotten in the C++11 standard, and will be added later).
//Once it's added, I can just remove this from here.
template<typename T, typename ...Args>
std::unique_ptr<T> make_unique( Args&& ...args )
{
	return std::unique_ptr<T>( new T( std::forward<Args>(args)... ) );
}

C++14 will add this (and some compilers might already have it implemented), and it'll basically be identical to the code above, except it'd be in the std:: namespace.

(It'll probably also include a tiny speed optimization, like std::make_shared does, by allocating the unique_ptr and the new T in a single allocation).

Then you get this code, which is (I think) fully safe:


m_clients.emplace_back(make_unique<Client>());

.

[Edit:] The possible leak occurs when you have multiple parameters, not in your single-parameter case (my mistake!):


void funcA(std::unique_ptr<Client>(new Client()), funcB());

If 'new Client()' is called, then funcB() is called before 'new Client()' is passed to the unique_ptr's constructor, and 'funcB()' throws, then the memory of Client() leaks. See here which then links here, which is basically why make_shared() (and soon, make_unique()) exists.

std::unique_ptr's constructor cannot throw an exception (unless templated with a deleter that can throw, but doing so is a violation of the unique_ptr template argument requirements). However, emplace_back() can throw an exception when allocating memory for the vector, so you still have two potential throw points.

Function calls are sequence points. All side effects of expressions that are used as arguments to that function call must be completed before the function call begins (unless such expressions involve other threads). In this case, that means that the result of the new expression must be known before the call to emplace_back begins. emplace_back() gives the exception guarantee that if an exception is thrown by anything other than a copy constructor, a move constructor, assignment operator or move assignment operator then there are no effects. std::unique_ptr will not throw for any of those unless you do something silly like make your Client's destructor throw. So if there is a memory allocation exception, nothing will happen to the vector and your temporary std::unique_ptr will clean up your new'd pointer as part of its destruction.

std::unique_ptr's constructor cannot throw an exception (unless templated with a deleter that can throw, but doing so is a violation of the unique_ptr template argument requirements). However, emplace_back() can throw an exception when allocating memory for the vector, so you still have two potential throw points.

Function calls are sequence points. All side effects of expressions that are used as arguments to that function call must be completed before the function call begins (unless such expressions involve other threads). In this case, that means that the result of the new expression must be known before the call to emplace_back begins. emplace_back() gives the exception guarantee that if an exception is thrown by anything other than a copy constructor, a move constructor, assignment operator or move assignment operator then there are no effects. std::unique_ptr will not throw for any of those unless you do something silly like make your Client's destructor throw. So if there is a memory allocation exception, nothing will happen to the vector and your temporary std::unique_ptr will clean up your new'd pointer as part of its destruction.

So, after reading a bit and thinking about what you said I came to the conclusion I actually was understanding emplace_back wrong. I didn't realize the "normal" form is to pass it the pure arguments for the object to be constructed inside.

So, looking at my code, seems like it calls new, then fully constructs the unique_ptr(which wraps the new'd object safely) then internally it forwards that temporary unique_ptr and constructs the internal version using the move constructor, which is why it's safe. Least I think? Maybe I'm wrong there.

Assuming I am correct that means that behavior wise it is essentially not really any different from just calling push_back with the same line, right? Since it would call the move constructor for the unique_ptr in both cases.

EDIT: Assuming that assumption is correct, does that mean just passing new Client to the vector would be unsafe? Since if the vector throws the new'd memory would be lost to the void?


(It'll probably also include a tiny speed optimization, like std::make_shared does, by allocating the unique_ptr and the new T in a single allocation).

What would it allocate for a unique_ptr?


(It'll probably also include a tiny speed optimization, like std::make_shared does, by allocating the unique_ptr and the new T in a single allocation).

What would it allocate for a unique_ptr?

Nothing apart from the type itself, I wasn't thinking clear. ph34r.png

So, looking at my code, seems like it calls new, then fully constructs the unique_ptr(which wraps the new'd object safely) then internally it forwards that temporary unique_ptr and constructs the internal version using the move constructor, which is why it's safe. Least I think? Maybe I'm wrong there.

That's pretty much what happens here.

Assuming I am correct that means that behavior wise it is essentially not really any different from just calling push_back with the same line, right? Since it would call the move constructor for the unique_ptr in both cases.

That's right.

EDIT: Assuming that assumption is correct, does that mean just passing new Client to the vector would be unsafe? Since if the vector throws the new'd memory would be lost to the void?

It would be unsafe in the general case. However, if the vector's capacity is greater than its size, then you're guaranteed not to have a reallocation for emplace_back(). So if you call reserve() first then you could emplace_back() without the temporary safely.

This topic is closed to new replies.

Advertisement