Re: [boost] ScopeExit review

From: Maxim Yanchenko (maximyanchenko_at_[hidden])
Date: 2007-08-23 02:17:18

Hi Andrey,

Andrey Semashev <andysem <at> mail.ru> writes:

> > I don't understand. Why can't you make a copy
> > right before the scope exit block?  I think that
> > for ScopeExit, always passing by reference is
> > correct.  At the very least it should be the default.
> 
> I can. But this is a burden since it should be done for almost all
> args I ever pass to ScopeExit. And I can easily forget to do so and
> get subtle errors I pointed out before. So this is by no way should be
> the default behavior.

Let's look at your example.
  std::set< int >::iterator it = my_set.insert(10);
  BOOST_SCOPE_EXIT((my_set)(it)(commit))
  {
    // Which one will be erased here?
    // Not necessarily the one we intended to.
    if (!commit)
      my_set.erase(it);
  }
  BOOST_SCOPE_EXIT_END

First of all, even while you state:
> the only exception to this is the "commit/dispose" flag. Playing otherwise 
may lead to subtle errors.

In your case my_set should be also sent be reference, and if it's passed by 
value, you'll get the mentioned subtle errors (you will delete from the copy of 
the set, and the original set will be unchanged).

So you're just switching from one set of subtle errors to another one.
But additionally to this another set of subtle errors you also break strong 
exception guarantee.

But if your copy ctor throws, you're in trouble at the line 1, because a new 
object is created here, and it's created after the insertion, and it can throw. 
It means that you will not reach ScopeExit block, i.e. your insertion will not 
be rolled back.

However, there are cases when copy ctor can fail, but assignment (or some other 
form of initialization/copying) can't (for example, ctor of std::vector can 
throw, but if you created and reserved enough memory successfully, assignment 
will not throw as there is no allocation).
So you need to create a temporary object before the insertion, and then 
initialize it in some no-throw fashion.

For example, consider the following code:
  extern vector<int>& mutate();
  vector<int> v = mutate(); // 1
  BOOST_SCOPE_EXIT((v)(commit))
The mutate() changes something and returns a reference to the changes.
Here in the line 1 ctor of v can throw, so you won't reach ScopeExit, and the 
mutation won't be rolled back.
The safe way here is to write
  vector<int> v; // 1
  v.reserve(MAX_MUTATION_NUMBER); // 2
  v = mutate(); // 3
  BOOST_SCOPE_EXIT((v)(commit))
Here the lines 1 and 2 can throw, and mutate() itself can also throw, but not 
the assignment. Therefore now, given that mutate() executed succesfully, you 
have guarantee that the ScopeExit block will be executed. 

To conclude, I don't think it worth to make default an option that breaks 
strong exception guarantee, if there is one that doesn't.

Thanks,
Maxim