Idiom for exception safety relating to constructor parameters

Question

I've been looking at some code I'm working on, and we have the equivalent of this:

AutoPtr<A> x;
...
/// x gets initialized
...
B* y = new B(x.Detach());

Where AutoPtr is our version of auto_ptr, and Detach() returns the owned pointer and resets itself. Also, B() takes ownership of x.

Now, I realized that this will leak x if new throws an std::bad_alloc, so I changed the code to this:

AutoPtr<A> x;
...
/// x gets initialized
...
B* y = new B(x.Get());
x.Detach();

But then I realized that if B() 'owns' the pointer, and an exception happens during its construction, it should take care of deleting the parameter itself (or should it?), so the x will get deleted twice, once by B(), and once by x's destructor.

Now, is there a C++ idiom that gets around this problem, for example, making code that calls constructors responsible for cleaning up parameters? Most code I've seen doesn't seem to do that...

hah, no it isn't, as it gets passed into another function that takes ownership of it on the next line. — Bwmat, Oct 10 '12 at 00:31

score 3 · Accepted Answer · answered Oct 10 '12 at 00:26

3

The obvious solution seems to be to pass a temporary AutoPtr<A> to the constructor of B:

AutoPtr<B> y(new B(AutoPtr<A>(x));

(this also adds resource control for the B* returned from new B()).

B's constructor would just call x.Detach() to initialize whatever it needs to initialize with the A*. If an exception occurs at any point, the AutoPtr<A> will release the object.

If you want to retain the A object managed by x in case of an exception, you can pass a AutoPtr<A>& to the constructor of B instead.

answered Oct 10 '12 at 00:26

Dietmar Kühl

150,225
13
225
380

I don't understand the reasoning for passing a copy of the `AutoPtr` rather than by reference. – Mooing Duck Oct 10 '12 at 00:27
Yeah, this makes sense. Unfortunately this code has constructors accepting raw owning pointers everywhere, so I doubt this is going to be implemented, as the probability of a bad_alloc isn't really worth the effort. – Bwmat Oct 10 '12 at 00:30
@Bwmat: At least _add_ this constructor, and begin to use it. Better safe sometimes than safe never. – Mooing Duck Oct 10 '12 at 00:31
1

@MooingDuck: The best choice would be to add two constructors: one taking the `AutoPtr` by reference and the other one by value. For the minimal change, i.e., just add one constructor, I would opt for the one taking the `AutoPtr ` by value because this avoids the need to name a variable when there is only a temporary, e.g., when the pointer is returned from a function. Since copying an `std::auto_ptr` and presumable also an `AutoPtr` is cheap it comes down to whether the object is typically named or not which is easier to use. – Dietmar Kühl Oct 10 '12 at 06:48

score 3 · Answer 2 · answered Oct 10 '12 at 00:28

3

...it should take care of deleting the parameter itself (or should it?)

No, it should not.

B doesn't exist until the constructor completes, and if it doesn't exist it shouldn't claim ownership of anything (to a point; if the constructor itself did something, that needs to be safe as well).

The C++ idiom is to not use raw pointers for ownership (including for y)! B should accept the AutoPtr as an argument so the caller can give up ownership this way. This is the goal of std::unique_ptr and std::move:

std::unique_ptr<A> x;

std::unique_ptr<B> y(new B(std::move(x)));

Also note that really new should not be used like this as well; instead use the make_* utilities:

auto y = std::make_unique<B>(std::move(x));

But this is missing currently as oversight.

answered Oct 10 '12 at 00:28

GManNickG

494,350
52
494
543

I really wish we could use std::unique_ptr, but C+11 isn't happening over here for a _while_ – Bwmat Oct 10 '12 at 00:33
@Bwmat: `auto_ptr` works for this purpose. Your `AutoPtr` ought to as well. – Mooing Duck Oct 10 '12 at 00:33
I was thinking about move semantics, actually. I guess they're not really neccessary though. – Bwmat Oct 10 '12 at 00:35

score 0 · Answer 3 · answered Oct 10 '12 at 00:29

0

Something like this, may be:

B* y = new B();
y->Attach(x.Detach());

or

B* y = new B();
(*y) = x;

answered Oct 10 '12 at 00:29

qehgt

2,972
1
22
36

Two phase initialization is bad – Mooing Duck Oct 10 '12 at 00:30
@MooingDuck dynamic allocation of `AutoPtr` is bad, actually – qehgt Oct 10 '12 at 00:31
1

Feel free to correct me if I'm wrong, but I don't see that anyone, including the OP, has suggested or implied that dynamic allocation of an `AutoPtr` would be good. – Mooing Duck Oct 10 '12 at 00:32

Idiom for exception safety relating to constructor parameters

3 Answers3