8

Let's say I have a class.

class BigData {...};
typedef boost::shared_ptr<BigData> BigDataPtr; 

Then I do:

BigDataPtr bigDataPtr(new BigData());

Later on after I am done with my object and I am sure there no other users for the object.

Is it safe to do the following:

bigDataPtr->~BigDataPtr();
new (&*bigDataPtr) BigData;

Would this let me reset the object without any additional allocations?

Jesse Good
  • 50,901
  • 14
  • 124
  • 166
Nathan Doromal
  • 3,437
  • 2
  • 24
  • 25
  • 1
    What's wrong with `*bigDataPtr = BigData();`? – jrok Apr 03 '13 at 20:56
  • Quite similar question, but I do not consider as duplicate: http://stackoverflow.com/questions/1124634/c-call-destructor-and-then-constructor-resetting-an-object – PiotrNycz Apr 03 '13 at 20:57
  • 3
    Did you mean `bigDataPtr->~BigData();`? Also, why not simply provide some `reset` method for `BigData`? – zch Apr 03 '13 at 20:59
  • @jrok I think I would end up making a temporary object in that case. Not sure if the compiler would optimize it out or not. – Nathan Doromal Apr 03 '13 at 20:59
  • 1
    @NateDoromal it would make a temp, but with move-assignment in C++11 it could be pretty efficient. With placement knew, you have destruct+construct. With jroks simple alternative you have construct+ moveassign+destruct(empty). it may not matter in the long run. Good mental floss. – WhozCraig Apr 03 '13 at 21:05
  • 2
    Generally speaking, this is a bad idea. It's unfortunate that the language standard uses this as an example in a discussion of object lifetime. There are far too many pitfalls for this to be a useful idiom. – Pete Becker Apr 03 '13 at 21:40
  • Everyone always thinks that there are no other users for an object... until there are. This is half the reason shared pointers were made - so you don't have to worry about this whole class of potential bugs. – Robert Mason Apr 03 '13 at 22:01

4 Answers4

6

There are a few ways to go about this. You can use placement new, and this is guaranteed to be safe for two reasons:

  1. You have already allocated the memory for the object, so you know it’s sized and aligned correctly.

  2. shared_ptr is non-invasive; its sole responsibility is to count references and call the deleter when necessary.

However, consider what can happen if reconstruction of the object fails—i.e., throws an exception:

bigDataPtr->~BigDataPtr();
new (bigDataPtr.get()) BigData;

Then you have a problem: the deleter can be called on a non-constructed object, leading almost certainly to undefined behaviour. I say “almost” because the deleter could be a no-op, in which case all would be well.

Safer, I think, would be to move a new value into the existing object:

*bigDataPtr = BigData(42);

Or add a reset() member function to BigData:

bigDataPtr->reset(42);

Then it’s explicit what your real intent is, and you don’t need to be as concerned about object lifetimes.

Jon Purdy
  • 53,300
  • 8
  • 96
  • 166
  • Adding a `reset` function is a good idea, but then the question still stands, how to implement that function. The questioner's code is (at least superficially, ignoring all the horrible caveats about how it can go wrong) one way to do that. – Steve Jessop Apr 03 '13 at 22:31
  • @SteveJessop: Sure. In the past I’ve even made a templated `renew` function, something like `template void renew(T* const p, Args&&... args) { p->~T(); new (p) (std::forward(args)...); }` but it’s not really possible to make it exception-safe. – Jon Purdy Apr 04 '13 at 01:41
2

It is safe if BigData constructor and destructor do not throw exceptions and bigDataPtr is not shared between threads and no pointers or references exist to dynamically allocated members of BigData (if any).

If the destructor throws an exception you may end up with a partially destroyed object (throwing destructors are not generally recommended and standard containers require that destructors of elements do not throw).

If the constructor throws you may end up destroying the object but not constructing a new one.

If bigDataPtr is shared between threads that may also lead to a race condition unless a locking discipline is used.

If code elsewhere takes references or pointers to dynamically allocated members of BigData, when it creates a new BigData its dynamically allocated members may be allocated at other addresses, so existing pointers and references to the members become invalid.

If you are concerned with dubious dereference in new (&*bigDataPtr) BigData; statement use a plain pointer instead:

BigData* p = bigDataPtr.get();
p->~BigData();
new (p) BigData;
Maxim Egorushkin
  • 131,725
  • 17
  • 180
  • 271
  • "throwing destructors are not generally recommended and standard containers require that destructors of elements do not throw" - actually, it's the language that prohibits throwing destructors - if an exception escapes a destructor `std::terminate` is called. – Matteo Italia Jun 06 '16 at 01:34
  • 1
    @MatteoItalia That is not entirely true. An exception thrown by a destructor is handled normally. The problem occurs when an object is destructed during stack unwinding, ie when an exception has already been thrown and has not been caught yet. Then the program does call `std::terminate`. – patatahooligan Sep 30 '19 at 10:16
  • 1
    @patatahooligan yep it's a bit more complicated than how I wrote it; what you wrote is what happened by default in C++03; since C++11 destructors are by default `noexcept`, so if they throw `std::terminate` is called; if you mark them with `noexcept(false)` (or if you inherit from a class that has a destructor marked as such) you go back to the old behavior you described. – Matteo Italia Sep 30 '19 at 11:58
  • @MatteoItalia You're right. I forgot about the implicit `noexcept`. Your initial comment is true in the default case. – patatahooligan Sep 30 '19 at 12:24
2

Yes it is normally safe. (Nod to Maxim Yegorushkin's observation about a throwing edge case)

Note the typo message below

Boost defines the dereference and -> operators as

template<class T>
typename boost::detail::sp_dereference< T >::type boost::shared_ptr< T >::operator* () const;

template<class T>
typename boost::detail::sp_member_access< T >::type boost::shared_ptr< T >::operator-> () const;

When those detail bits are resolved, you have this

template<class T>
T & boost::shared_ptr< T >::operator* () const

template<class T>
T * boost::shared_ptr< T >::operator-> () const 

So you are dealing with the pointed-to object directly. There are no proxies or other constructs that may interfere with what you're attempting.

As the pointed-to data is concerned, your code:

bigDataPtr->~BigDataPtr();
new (&*bigDataPtr) BigData;

May have a typo. But if you intended:

bigDataPtr->~BigData();
new (&*bigDataPtr) BigData;

It will resolve to

(BigData pointer)->~BigData();
new (&(BigData reference)) BigData;

This is legal, and you are correct that it would avoid the additional allocation normally incurred with an assignment.

Drew Dormann
  • 59,987
  • 13
  • 123
  • 180
  • It's also important to be aware that it depends on the type `BigData`. It results in UB if there are any `const` or reference non-static data members in `BigData` (or its bases or members, recursively). – Steve Jessop Sep 01 '13 at 09:33
1

Firstly, if the constructor throws and the class is not trivially destructible then you have a problem, since the shared_ptr "wants" to delete it, which would provoke UB.

So you must deal with that, either by using a nothrow constructor or by catching any exception and preventing the smart pointer from deleting the object. Since shared_ptr doesn't have a release() function, that's easier said than done. You could call terminate() if all else fails, but that won't make you popular with your users.

If there are no other references to the object, then it will work provided that the class has no const or reference non-static data members (including members-of-members). The reason is 3.8/7:

If, after the lifetime of an object has ended and before the storage which the object occupied is reused or released, a new object is created at the storage location which the original object occupied, a pointer that pointed to the original object ... can be used to manipulate the new object, if ... the type of the original object is not const-qualified, and, if a class type, does not contain any non-static data member whose type is const-qualified or a reference type ...

Note that the shared_ptr holds just such a pointer, which it will use to manipulate the new object, which is UB if any of the conditions in 3.8/7 is broken. The only one that might be broken is this one, you've covered the rest with what you've said about your code. In particular, it's required that you created the original object as an instance of BigData, not a class derived from BigData, because the new object is required to have the same most-derived type as the old one.

There are usually more robust ways to reset an object than this. For example, implement operator= (copy- or move assignment operator) and then write *bigDataPtr = BigData(). Of course that might not be quite as fast.

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699