Advertisement Jump to content
Sign in to follow this  
Satharis

Unique_ptr and emplace_back.

This topic is 1864 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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?

Edited by Satharis

Share this post


Link to post
Share on other sites
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.

Edited by Servant of the Lord

Share this post


Link to post
Share on other sites

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. 

Share this post


Link to post
Share on other sites

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?

Edited by Satharis

Share this post


Link to post
Share on other sites


(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?

Share this post


Link to post
Share on other sites

 


(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

Edited by Servant of the Lord

Share this post


Link to post
Share on other sites

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. Edited by SiCrane

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!