The simplest and neatest c++11 ScopeGuard

Question

I'm attempting to write a simple ScopeGuard based on Alexandrescu concepts but with c++11 idioms.

namespace RAII
{
    template< typename Lambda >
    class ScopeGuard
    {
        mutable bool committed;
        Lambda rollbackLambda; 
        public:

            ScopeGuard( const Lambda& _l) : committed(false) , rollbackLambda(_l) {}

            template< typename AdquireLambda >
            ScopeGuard( const AdquireLambda& _al , const Lambda& _l) : committed(false) , rollbackLambda(_l)
            {
                _al();
            }

            ~ScopeGuard()
            {
                if (!committed)
                    rollbackLambda();
            }
            inline void commit() const { committed = true; }
    };

    template< typename aLambda , typename rLambda>
    const ScopeGuard< rLambda >& makeScopeGuard( const aLambda& _a , const rLambda& _r)
    {
        return ScopeGuard< rLambda >( _a , _r );
    }

    template<typename rLambda>
    const ScopeGuard< rLambda >& makeScopeGuard(const rLambda& _r)
    {
        return ScopeGuard< rLambda >(_r );
    }
}

Here is the usage:

void SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptions() 
{
   std::vector<int> myVec;
   std::vector<int> someOtherVec;

   myVec.push_back(5);
   //first constructor, adquire happens elsewhere
   const auto& a = RAII::makeScopeGuard( [&]() { myVec.pop_back(); } );  

   //sintactically neater, since everything happens in a single line
   const auto& b = RAII::makeScopeGuard( [&]() { someOtherVec.push_back(42); }
                     , [&]() { someOtherVec.pop_back(); } ); 

   b.commit();
   a.commit();
}

Since my version is way shorter than most examples out there (like Boost ScopeExit) i'm wondering what specialties i'm leaving out. Hopefully i'm in a 80/20 scenario here (where i got 80 percent of neatness with 20 percent of lines of code), but i couldn't help but wonder if i'm missing something important, or is there some shortcoming worth mentioning of this version of the ScopeGuard idiom

thanks!

Edit I noticed a very important issue with the makeScopeGuard that takes the adquire lambda in the constructor. If the adquire lambda throws, then the release lambda is never called, because the scope guard was never fully constructed. In many cases, this is the desired behavior, but i feel that sometimes a version that will invoke rollback if a throw happens is desired as well:

//WARNING: only safe if adquire lambda does not throw, otherwise release lambda is never invoked, because the scope guard never finished initialistion..
template< typename aLambda , typename rLambda>
ScopeGuard< rLambda > // return by value is the preferred C++11 way.
makeScopeGuardThatDoesNOTRollbackIfAdquireThrows( aLambda&& _a , rLambda&& _r) // again perfect forwarding
{
    return ScopeGuard< rLambda >( std::forward<aLambda>(_a) , std::forward<rLambda>(_r )); // *** no longer UB, because we're returning by value
}

template< typename aLambda , typename rLambda>
ScopeGuard< rLambda > // return by value is the preferred C++11 way.
makeScopeGuardThatDoesRollbackIfAdquireThrows( aLambda&& _a , rLambda&& _r) // again perfect forwarding
{
    auto scope = ScopeGuard< rLambda >(std::forward<rLambda>(_r )); // *** no longer UB, because we're returning by value
    _a();
    return scope;
}

so for completeness, i want to put in here the complete code, including tests:

#include <vector>

namespace RAII
{

    template< typename Lambda >
    class ScopeGuard
    {
        bool committed;
        Lambda rollbackLambda; 
        public:

            ScopeGuard( const Lambda& _l) : committed(false) , rollbackLambda(_l) {}

            ScopeGuard( const ScopeGuard& _sc) : committed(false) , rollbackLambda(_sc.rollbackLambda) 
            {
                if (_sc.committed)
                   committed = true;
                else
                   _sc.commit();
            }

            ScopeGuard( ScopeGuard&& _sc) : committed(false) , rollbackLambda(_sc.rollbackLambda)
            {
                if (_sc.committed)
                   committed = true;
                else
                   _sc.commit();
            }

            //WARNING: only safe if adquire lambda does not throw, otherwise release lambda is never invoked, because the scope guard never finished initialistion..
            template< typename AdquireLambda >
            ScopeGuard( const AdquireLambda& _al , const Lambda& _l) : committed(false) , rollbackLambda(_l)
            {
               std::forward<AdquireLambda>(_al)();
            }

            //WARNING: only safe if adquire lambda does not throw, otherwise release lambda is never invoked, because the scope guard never finished initialistion..
            template< typename AdquireLambda, typename L >
            ScopeGuard( AdquireLambda&& _al , L&& _l) : committed(false) , rollbackLambda(std::forward<L>(_l))
            {
                std::forward<AdquireLambda>(_al)(); // just in case the functor has &&-qualified operator()
            }


            ~ScopeGuard()
            {
                if (!committed)
                    rollbackLambda();
            }
            inline void commit() { committed = true; }
    };


    //WARNING: only safe if adquire lambda does not throw, otherwise release lambda is never invoked, because the scope guard never finished initialistion..
    template< typename aLambda , typename rLambda>
    ScopeGuard< rLambda > // return by value is the preferred C++11 way.
    makeScopeGuardThatDoesNOTRollbackIfAdquireThrows( aLambda&& _a , rLambda&& _r) // again perfect forwarding
    {
        return ScopeGuard< rLambda >( std::forward<aLambda>(_a) , std::forward<rLambda>(_r )); // *** no longer UB, because we're returning by value
    }

    template< typename aLambda , typename rLambda>
    ScopeGuard< rLambda > // return by value is the preferred C++11 way.
    makeScopeGuardThatDoesRollbackIfAdquireThrows( aLambda&& _a , rLambda&& _r) // again perfect forwarding
    {
        auto scope = ScopeGuard< rLambda >(std::forward<rLambda>(_r )); // *** no longer UB, because we're returning by value
        _a();
        return scope;
    }

    template<typename rLambda>
    ScopeGuard< rLambda > makeScopeGuard(rLambda&& _r)
    {
        return ScopeGuard< rLambda >( std::forward<rLambda>(_r ));
    }

    namespace basic_usage
    {
        struct Test
        {

            std::vector<int> myVec;
            std::vector<int> someOtherVec;
            bool shouldThrow;
            void run()
            {
                shouldThrow = true;
                try
                {
                    SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesNOTRollbackIfAdquireThrows();
                } catch (...)
                {
                    AssertMsg( myVec.size() == 0 && someOtherVec.size() == 0 , "rollback did not work");
                }
                shouldThrow = false;
                SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesNOTRollbackIfAdquireThrows();
                AssertMsg( myVec.size() == 1 && someOtherVec.size() == 1 , "unexpected end state");
                shouldThrow = true;
                myVec.clear(); someOtherVec.clear();  
                try
                {
                    SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesRollbackIfAdquireThrows();
                } catch (...)
                {
                    AssertMsg( myVec.size() == 0 && someOtherVec.size() == 0 , "rollback did not work");
                }
            }

            void SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesNOTRollbackIfAdquireThrows() //throw()
            {

                myVec.push_back(42);
                auto a = RAII::makeScopeGuard( [&]() { HAssertMsg( myVec.size() > 0 , "attempt to call pop_back() in empty myVec"); myVec.pop_back(); } );  

                auto b = RAII::makeScopeGuardThatDoesNOTRollbackIfAdquireThrows( [&]() { someOtherVec.push_back(42); }
                                    , [&]() { HAssertMsg( myVec.size() > 0 , "attempt to call pop_back() in empty someOtherVec"); someOtherVec.pop_back(); } );

                if (shouldThrow) throw 1; 

                b.commit();
                a.commit();
            }

            void SomeFuncThatShouldBehaveAtomicallyInCaseOfExceptionsUsingScopeGuardsThatDoesRollbackIfAdquireThrows() //throw()
            {
                myVec.push_back(42);
                auto a = RAII::makeScopeGuard( [&]() { HAssertMsg( myVec.size() > 0 , "attempt to call pop_back() in empty myVec"); myVec.pop_back(); } );  

                auto b = RAII::makeScopeGuardThatDoesRollbackIfAdquireThrows( [&]() { someOtherVec.push_back(42); if (shouldThrow) throw 1; }
                                    , [&]() { HAssertMsg( myVec.size() > 0 , "attempt to call pop_back() in empty someOtherVec"); someOtherVec.pop_back(); } );

                b.commit();
                a.commit();
            }
        };
    }
}

One thing you're leaving out is that this code doesn't quite compile. The declarations of the guard variables are missing template parameters. — R. Martinho Fernandes, Apr 22 '12 at 17:36
@R.MartinhoFernandes shouldn't the compiler be able to deduce from the arguments? these are lambdas, so as you know their type is opaque — lurscher, Apr 22 '12 at 17:42
@R.MartinhoFernandes so i keep the temporaries in const references, and changed the commited member to mutable so its modifiable in a const object — lurscher, Apr 22 '12 at 17:50
@lursher **I am not your compiler**. The code still doesn't compile *for the same reasons*. You could avoid wasting everyone's time if you tried to compile your code before posting it. — R. Martinho Fernandes, Apr 22 '12 at 17:52
@lurscher, if I were you, I wouldn't make `commited` mutable, but I'd remove `const` from `commit()` - after all, `commit()` changes the state of the object - and that change is important; why the function that changes important part of object state would be marked `const`? — Griwes, Apr 22 '12 at 17:52
@Griwes, because a reference to a temporary can only be held with const references. This is explained in the original Alexandrescu article about ScopeGuard — lurscher, Apr 22 '12 at 18:02
@Griwes, the references are `a` and `b` and they reference two temporary objects — lurscher, Apr 22 '12 at 18:06
@lurscher, wait, maybe I forgot about some important thing, but what does the fact that `a` and `b` are temporaries have to do with `inline void commit() const { committed = true; }`? — Griwes, Apr 22 '12 at 18:08
@Griwes, if commit() would not be a const method, i could not call it on those temporary references. If committed would not be mutable, commit() could not be a const method — lurscher, Apr 22 '12 at 18:11
@Griwes, this is basically an elaborate trick to avoid copying by value, before c++11 move constructors, people had to implement move semantics on the scope guards to avoid having temporaries call release callbacks multiple times. This was a shortcut to avoid that complexity — lurscher, Apr 22 '12 at 18:13
I just posted a [much simpler version](http://stackoverflow.com/a/28413370/168683) based on the same article. Note that I don't need generator functions. — Fozi, Feb 09 '15 at 15:38
I didn't know that this idiom is called Scoped Guard. So I created a similar question: https://stackoverflow.com/q/48842770/5447906 (solutions from there may be helpful). — anton_rh, Feb 18 '18 at 04:58

score 37 · Answer 1 · edited Oct 15 '19 at 09:52

37

Even shorter: I don't know why you guys insist on putting the template on the guard class.

#include <functional>

class scope_guard {
public: 
    template<class Callable> 
    scope_guard(Callable && undo_func) try : f(std::forward<Callable>(undo_func)) {
    } catch(...) {
        undo_func();
        throw;
    }

    scope_guard(scope_guard && other) : f(std::move(other.f)) {
        other.f = nullptr;
    }

    ~scope_guard() {
        if(f) f(); // must not throw
    }

    void dismiss() noexcept {
        f = nullptr;
    }

    scope_guard(const scope_guard&) = delete;
    void operator = (const scope_guard&) = delete;

private:
    std::function<void()> f;
};

Note that it is essential that the cleanup code does not throw, otherwise you get in similar situations as with throwing destructors.

Usage:

// do step 1
step1();
scope_guard guard1 = [&]() {
    // revert step 1
    revert1();
};

// step 2
step2();
guard1.dismiss();

My inspiration was the same DrDobbs article as for the OP.

Edit 2017/2018: After watching (some of) Andrei's presentation that André linked to (I skipped to the end where it said "Painfully Close to Ideal!") I realized that it's doable. Most of the time you don't want to have extra guards for everything. You just do stuff, and in the end it either succeeds or rollback should happen.

Edit 2018: Added execution policy which removed the necessity of the dismiss call.

#include <functional>
#include <deque>

class scope_guard {
public:
    enum execution { always, no_exception, exception };

    scope_guard(scope_guard &&) = default;
    explicit scope_guard(execution policy = always) : policy(policy) {}

    template<class Callable>
    scope_guard(Callable && func, execution policy = always) : policy(policy) {
        this->operator += <Callable>(std::forward<Callable>(func));
    }

    template<class Callable>
    scope_guard& operator += (Callable && func) try {
        handlers.emplace_front(std::forward<Callable>(func));
        return *this;
    } catch(...) {
        if(policy != no_exception) func();
        throw;
    }

    ~scope_guard() {
        if(policy == always || (std::uncaught_exception() == (policy == exception))) {
            for(auto &f : handlers) try {
                f(); // must not throw
            } catch(...) { /* std::terminate(); ? */ }
        }
    }

    void dismiss() noexcept {
        handlers.clear();
    }

private:
    scope_guard(const scope_guard&) = delete;
    void operator = (const scope_guard&) = delete;

    std::deque<std::function<void()>> handlers;
    execution policy = always;
};

Usage:

scope_guard scope_exit, scope_fail(scope_guard::execution::exception);

action1();
scope_exit += [](){ cleanup1(); };
scope_fail += [](){ rollback1(); };

action2();
scope_exit += [](){ cleanup2(); };
scope_fail += [](){ rollback2(); };

// ...

edited Oct 15 '19 at 09:52

andreee

4,459
22
42

answered Feb 09 '15 at 15:34

Fozi

4,973
1
32
56

4

"I don't know why you guys insist on putting the template on the guard class" - we didn't think to hide it behind std::function's type erasure! – M.M Aug 20 '15 at 04:32
The best so far, IMHO – Levi Haskell Nov 05 '15 at 21:39
12

Do note that if the captured variables don't fit within the in-place buffer of `std::function`, it will incur dynamic allocation. – mpark Feb 09 '16 at 04:39
1

@mpark If this is a concern you could use function pointers, e.g. you could write `scope_guard guard1 = revert1;`. – Fozi Feb 09 '16 at 14:38
@Fozi of course you can do that if there are no capture variables. I mean in the case where there are captured variables, preserving the true type of the lambda avoids incurring dynamic allocations. – mpark Feb 10 '16 at 02:43
3

Also, std::function uses a level of indirection and thus has more overhead. – Brandon Kohn Jul 15 '16 at 12:06
1

`throw()` is deprecated in C++11, `noexcept` replaces it. – Zitrax Dec 09 '16 at 13:03
I think `function::operator=(nullptr_t)` isn't noexcept pre-C++17. So, pre-C++17, your move constructor can throw at `other.f = nullptr`, which would likely prevent the clean up function from ever executing (it depends on the state of the moved-from other.f). Also, dismiss() can terminate() because `f = nullptr` can (try to) throw. Maybe these aren't issues in practice, but you could get around them by using a separate flag instead of a null value. – tgnottingham Mar 24 '17 at 22:58
1

Also, I think scope_guard's constructor and scope_guards' operator+= can throw, preventing you from registering cleanup functions that need to run. I'm not sure any of the other implementations get around this. – tgnottingham Mar 24 '17 at 23:27
@tgnottingham Despite of what cppreference.com claims, `funtion::operator=(nullptr_t)` is noexcept since C++11, see 20.8.11.2.1 `[func.wrap.func.con]`. You are right with the move constructor and emplace, but realistically it's not going to fail. If you want to be sure you can add the cleanup before the operation and check in the cleanup if the operation was actually done. – Fozi Mar 25 '17 at 00:00
@tgnottingham Ah, sorry I confused the constructor and the assignment. But still, unless the `Callable` destructor throws (in which case there is nothing that could be done) I don't see how the `nullptr_t` assignment can throw. I think that's why they added it in C++17, because all implementations were already de-facto noexcept. – Fozi Mar 25 '17 at 00:26
Wouldn't private inheritance be better in this scenario? – Daniel Langr Feb 01 '18 at 13:12
@DanielLangr Up to you: for me, `operator +=` is just a convenience method. I think there is nothing wrong with using any of the other `deque` methods to add or remove or iterate over the handlers, so I leave it accessible. You could make it private if you want to limit the interface. – Fozi Feb 01 '18 at 15:01
@Fozi All right, see your point. I like this solution with type erasure due to `std::function`. One can make, e.g., an external `vector` of `scope_guard`s, which wasn't possible with templated definitions. – Daniel Langr Feb 01 '18 at 15:05
`std::function` constructor may throw. If it throws, the cleanup code won't be executed. – anton_rh Feb 17 '18 at 15:43
@anton_rh There, fixed. The problem now is that *theoretically* the `undo_func` could be in an undefined moved state when the catch block is executed, but *practically* the only exception encountered here should be `bad_alloc` which should be thrown before moving. – Fozi Feb 18 '18 at 00:39
@Fozi, Ok, your fix is accepted. If you use lambdas with all variables captured by reference for `undo_func`, the move constructor shouldn't throw, so `bad_alloc` is only possible case. The only thing I still don't like is that `std::function` may involve dynamic allocations which cause performance penalty. I have a solution that doesn't use `std::function` (https://stackoverflow.com/a/48842771/5447906), but it has more complex implementation. – anton_rh Feb 18 '18 at 04:55
I'm wondering if I should remove the first scope_guard now that the second scope_guard is able to do anything the first one can, even though the second scope_guard is fatter due to the `std::deque`. – Fozi Feb 28 '18 at 05:05
Is the code missing a move-assignment operator? I seem to get failures to use the copy-assignment operator when attempting to do move-assignment. Am I doing something wrong? – starball Sep 09 '22 at 18:42
@davidfong Works for me... https://ideone.com/hmuN8u `scope_guard b = std::move(a);` – Fozi Sep 17 '22 at 15:25
You seem to be forwarding `undo_func` and `func` in `try` blocks, but if an exception is then thrown you then use these functions in the `catch` blocks. I cannot see how this is permitted without risking undefined behaviour. – Alex W Jun 14 '23 at 13:27
@AlexW In what situation would the move succeed (or the moved callable invalidated) and an exception thrown? – Fozi Jun 15 '23 at 21:04
@Fozi The move constructor of `std::function` is not marked `noexcept` for C++11, C++14, and C++17. Additionally, [cpp-reference](https://en.cppreference.com/w/cpp/utility/functional/function/function) states that the value moved in is left 'in a valid but unspecified state' after the call. I may be missing something(?), but this is why I thought it's probably not wise to use it after attempted the call. – Alex W Jun 15 '23 at 23:10
@AlexW I appreciate your comment, it's good to ask! :) Note that it is using `forward`, not `move`, so it is only moved for rvalue references. And yes, after the move `other` is left in an unspecified state - it is only meant to be destroyed. That's all ok and expected. The weird part is when there *is* an exception. The last part of moving should not cause an exception, you just copy the pointer. So the only reasonable place where an exception is thrown is when the target is prepared, e.g. out of memory. In that case `func` has not been moved yet so it should still be valid and can be called. – Fozi Jun 17 '23 at 02:09
@Fozi But do we get a guarantee of that from the spec or is it just a sensible guess? It seems to me that the moment you pass an rvalue reference to something you should stop using it, regardless of success, unless it explicitly documents otherwise (e.g. by stating a strong exception guarantee). – Alex W Jun 17 '23 at 21:31
@AlexW You are right, there is no guarantee from the spec, but I think this is more then just a sensible guess. An implementation would have to be borderline malicious to throw at that spot. However, if you want to be sure you can remove the `forward` call from the `+=` operator, that is make it `handlers.push_front(func);`. You will take a performance penalty though. – Fozi Jun 18 '23 at 00:42
@Fozi Okay, well it sounds like I probably wasn't missing anything, thanks for getting back to me. In practice I suspect you're right, and I can't see any way this issue would occur precisely, but without a spec guarantee it's hard to feel confident that every compiler (including for unusual architectures etc.) will work as intended but for standard compilers/etc. and the burden for failure would be on the dev. But I'm confident the code will be fine for the use-cases most will ever have. Thanks! – Alex W Jun 18 '23 at 10:37

R. Martinho Fernandes · Accepted Answer · 2014-08-21T22:56:16.700

23

Boost.ScopeExit is a macro that needs to work with non-C++11 code, i.e. code that has no access to lambdas in the language. It uses some clever template hacks (like abusing the ambiguity that arises from using < for both templates and comparison operators!) and the preprocessor to emulate lambda features. That's why the code is longer.

The code shown is also buggy (which is probably the strongest reason to use an existing solution): it invokes undefined behaviour due to returning references to temporaries.

Since you're trying to use C++11 features, the code could be improved a lot by using move semantics, rvalue references and perfect-forwarding:

template< typename Lambda >
class ScopeGuard
{
    bool committed; // not mutable
    Lambda rollbackLambda; 
    public:


        // make sure this is not a copy ctor
        template <typename L,
                  DisableIf<std::is_same<RemoveReference<RemoveCv<L>>, ScopeGuard<Lambda>>> =_
        >
        /* see http://loungecpp.net/w/EnableIf_in_C%2B%2B11
         * and http://stackoverflow.com/q/10180552/46642 for info on DisableIf
         */
        explicit ScopeGuard(L&& _l)
        // explicit, unless you want implicit conversions from *everything*
        : committed(false)
        , rollbackLambda(std::forward<L>(_l)) // avoid copying unless necessary
        {}

        template< typename AdquireLambda, typename L >
        ScopeGuard( AdquireLambda&& _al , L&& _l) : committed(false) , rollbackLambda(std::forward<L>(_l))
        {
            std::forward<AdquireLambda>(_al)(); // just in case the functor has &&-qualified operator()
        }

        // move constructor
        ScopeGuard(ScopeGuard&& that)
        : committed(that.committed)
        , rollbackLambda(std::move(that.rollbackLambda)) {
            that.committed = true;
        }

        ~ScopeGuard()
        {
            if (!committed)
                rollbackLambda(); // what if this throws?
        }
        void commit() { committed = true; } // no need for const
};

template< typename aLambda , typename rLambda>
ScopeGuard< rLambda > // return by value is the preferred C++11 way.
makeScopeGuard( aLambda&& _a , rLambda&& _r) // again perfect forwarding
{
    return ScopeGuard< rLambda >( std::forward<aLambda>(_a) , std::forward<rLambda>(_r )); // *** no longer UB, because we're returning by value
}

template<typename rLambda>
ScopeGuard< rLambda > makeScopeGuard(rLambda&& _r)
{
    return ScopeGuard< rLambda >( std::forward<rLambda>(_r ));
}

edited Aug 21 '14 at 22:56

answered Apr 22 '12 at 18:27

R. Martinho Fernandes

228,013
71
433
510

R. Martinho, read the Herb Sutter blog posted by mirk, const references to temporaries is not undefined behavior – lurscher Apr 22 '12 at 18:44
11

@lurscher No, you go back and read it carefully. The functions there return by value. – R. Martinho Fernandes Apr 22 '12 at 18:48
1

right, right right... so this does not happen because the return type reference is already killing the temporary at return, thanks for opening my eyes, i wouldn't have notice that – lurscher Apr 22 '12 at 18:50
Exactly. The reference is immediately dangling the moment it's returned. – R. Martinho Fernandes Apr 22 '12 at 18:52
quick question: why you had to add `std::forward(_al)();`, whats the difference between that and simply doing `_al();` inside the move constructor? – lurscher Apr 23 '12 at 19:34
@lurscher just in case you use a function object like this: `struct foo { void operator()() && /* not the rvalue qualifier */; };`. – R. Martinho Fernandes Apr 23 '12 at 19:36
what is that rvalue qualifier? shouldn't that qualifier be in the return type? i haven't seen that usage of && before – lurscher Apr 23 '12 at 19:40
1

@lurscher There's a very comprehensive explanation here: http://stackoverflow.com/a/8610728/46642. It's not a feature you'll use everyday, but when writing generic code, it's good to be as generic as possible. :) – R. Martinho Fernandes Apr 23 '12 at 19:43
1

Couldn't you significantly simplify this by using non-template class and storing the cleanup function in std::function? – Shachar Shemesh Apr 26 '14 at 05:38
@ShacharShemesh: `std::function` is not a free abstraction. There is a penalty in both time and space for using it instead of a template parameter: in the worst case, you need to do a dynamic memory allocation. – David Stone Jun 05 '17 at 21:39
May be this is not important here, but lambda in the `Visual Studio 2015 Update 3` has very low rate bug related to the capture arguments. Sometimes what kind of arguments found uninitialized at moment of usage in a lambda and so leads to access violation. Because `Boost.ScopeExit` utilizes the same approach (arguments capture by the []-block) then it has the same bug. Workaround here is to replace the capture by explicit lambda parameter. – Andry Jun 06 '18 at 00:22
This has a bug: `Lambda` can bind to an lvalue reference so, in the move ctor, `std::move(that.rollbackLambda)` should use `std::forward` instead! This shows the importance of testing. For an extensively tested implementation see [this answer](https://stackoverflow.com/a/51866951) – ricab Aug 15 '18 at 22:13

score 17 · Answer 3 · answered May 08 '18 at 18:48

17

I use this works like a charm, no extra code.

shared_ptr<int> x(NULL, [&](int *) { CloseResource(); });

answered May 08 '18 at 18:48

stu

8,461
18
74
112

1

...alas, it allocates memory :-\ – C.M. Jan 08 '19 at 18:27
1

I didn't notice anything in the question that said "and it shouldn't allocate memory." There's very few things in c++ that aren't c that don't allocate memory. I've long since given up trying to do things well in c++, there's no point they've made such a mess of the language. CPUs are so wicked fast that I could care less about a little memory allocation for a built-in scope guard with no code added. But that's me, your mileage may vary. – stu Jan 10 '19 at 01:33
performance aside this could be important in certain contexts (like in a nothrow function), some people still write code that can handle `std::bad_alloc` (or at least tries to). – C.M. Jan 10 '19 at 04:03
that seems silly. linux at least can't fail memory allocations. If you run out of memory, the OOM killer will kill a process to free up memory. Probably yours. That problem was solved a long time ago. – stu Jan 11 '19 at 12:52
And if you were really worried about memory allocation and wanted a lot of control of your program, you'd use C not C++ – stu Jan 11 '19 at 12:53
1

Dude, you are too bitter -- lighten up, it isn't as bad as you think :) C++ is as good as C wrt program control. Linux can fail memory allocations (e.g. when running x32 processes in x64 environment, when process is rlimit-ed or when kernel is configured to not overcommit). And OOM killer is not a good reason to ignore OOMs. – C.M. Jan 11 '19 at 13:33
1

Not bitter, uptight. :-) The problem is the bar for what is considered good software nowadays is so low, it's hard to worry about the out of memory case when a lot of time the positive test case doesn't even work. – stu Jan 12 '19 at 14:59
lol... kinda true, depending on where you work. There are still places where (almost) correct code is still valued. Almost because [this](https://stackoverflow.com/questions/45497684/what-happens-if-throw-fails-to-allocate-memory-for-exception-object/45552806#45552806) makes it impossible to write correct code (that uses exceptions), have to fallback on external solutions (auto-restart, microservices, etc) – C.M. Jan 12 '19 at 20:03
and that's why you don't use exceptions. :-) c++ is just as broken as all the other broken software. :-) – stu Jan 13 '19 at 21:55
Nah... I like mechanism of exceptions -- it just needs some fixin'. C++ isn't really broken, but messy -- there is no denying that. :) – C.M. Jan 14 '19 at 15:23
You ever notice how if something throws an exception you don't expect, everything unwinds way farther than you expected and it's really hard to figure out what happened and where the flow of control went? exceptions are a good idea in concept but that's a showstopper for me. – stu Jan 19 '19 at 15:57
I tend to not to distinguish exceptions from each other -- i.e. I either expect _some/any_ exception or function is guaranteed _nothrow_. And structure my code along this logic. If you need to tell an exception from others (and make processing it a part of your normal execution flow) -- it is a good candidate for turning into return code. In terms of difficulties of determining what happened -- return codes are pretty difficult too. (e.g. windows HRESULT). Proper logging helps a lot here. – C.M. Jan 20 '19 at 16:25
...in fact logging can be implemented with scope guard and activate during stack unwinding (due to exception) only. This would certainly help with figuring out stuff, but would impose quite strict requirements on both scope guard and logging itself (nothrow, as little of footprint as possible, etc) – C.M. Jan 20 '19 at 18:27
Right, I'll buy that, but if there's 3 different things that can throw in one try catch block, and one of them does... it's usually not obvious which one was the culprit unless you have logging between each one anyway, no? Or you can guarantee that each throwing thing will generate a unique enough e.what() message to identify it. I prefer return codes, my code is littered with r = func(); if (r != 0) then return r; but at least I know exactly what went wrong. I don't actually use numbers, I have a Ret object that encapsulates a code and a message and logs, never fails to spot the problem. – stu Jan 23 '19 at 02:22
Fundamentally, it doesn't matter how error gets propagated (or whether it is a number or an object) -- both mechanisms have pros and cons. And in both cases cons can be addressed one way or another. In many way all this is about taste. – C.M. Jan 23 '19 at 15:23
agreed, most programming style choices are just that, choices based on opinions. But exceptions have screwed me over more times than any other feature, so I try and swear others off them before they suffer as I have. :-) – stu Jan 25 '19 at 02:00
Two notes: it is possible to get bad_alloc on Linux, just ask for too way to much ram!. Exceptions, without entering into bad/good, is not structured programming. – Adrian Maire Sep 08 '20 at 10:01
well yeah, if you malloc(-1) I wouldn't expect that to oom your program, but I was referring to excessive numbers of reasonable requests. And as for exceptions, it apparently doesn't matter if it's structured programming or not, people still preach it. So maybe everything isn't structured programming. – stu Sep 09 '20 at 12:15

score 15 · Answer 4 · answered Mar 08 '13 at 22:29

15

You might be interested in seeing this presentation by Andrei himself on his own taken on how to improve scopedguard with c++11

answered Mar 08 '13 at 22:29

André

167
1
2

14

I would likely vote for answer if you would also summarize Andrei's answer to the OP's question here... What happens if that link rots? – U007D Oct 27 '16 at 23:48
5

Yes summary needed - the link is a 70 minute long video presentation. – Zitrax Dec 09 '16 at 13:25
The implementation details of the ScopeGuard start at the 1:19:30 mark of the video. However the presenter, Andrei, also maintains the [Loki library](https://sourceforge.net/projects/loki-lib/files/Loki/), which includes a ScopeGuard implementation. It's worth a look. – cemdervis Jan 25 '18 at 12:33

score 15 · Answer 5 · answered Jun 17 '16 at 10:35

You can use std::unique_ptr for that purpose which implements the RAII pattern. For example:

vector<int> v{};
v.push_back(42);
unique_ptr<decltype(v), function<void(decltype(v)*)>>
    p{&v, [] (decltype(v)* v) { if (uncaught_exception()) { v->pop_back(); }}};
throw exception(); // rollback 
p.release(); // explicit commit

The deleter function from the unique_ptr p rolls the formerly inserted value back, if the scope was left while an exception is active. If you prefer an explicit commit, you can remove the uncaugth_exception() question in the deleter function and add at the end of the block p.release() which releases the pointer. See Demo here.

score 11 · Answer 6 · answered Apr 16 '20 at 04:29

11

Most of the other solutions involve a move of a lambda, e.g. by using the lambda argument to initialize a std::function or an object of type deduced from the lambda.

Here's one that is quite simple, and allows using a named lambda without moving it (requires C++17):

template<typename F>
struct OnExit
{
    F func;
    OnExit(F&& f): func(std::forward<F>(f)) {}
    ~OnExit() { func();  }
};

template<typename F> OnExit(F&& frv) -> OnExit<F>;

int main()
{
    auto func = []{ };
    OnExit x(func);       // No move, F& refers to func
    OnExit y([]{});       // Lambda is moved to F.
}

The deduction guide makes F deduce as lvalue reference when the argument is an lvalue.

answered Apr 16 '20 at 04:29

M.M

138,810
21
208
365

Sorry to answer your 4 year old question. But could you explain what `template OnExit(F&& frv) -> OnExit;``actually does ? Or where I find what actually happens there ? Or give me references where I can read it up? – Nextar May 01 '20 at 11:34
2

@Nextar look up "deduction guide" (I explain this in the last sentence) – M.M May 01 '20 at 13:09
I like this solution, but what is the advantage of not moving the lambda...? – j b May 05 '20 at 16:54
@jb if the lambda has captures then moving it might be expensive or incorrect – M.M May 06 '20 at 00:25

Erik van Velzen · Answer 7 · 2016-02-01T14:48:23.070

There's a chance this approach will be standardized in C++17 or in the Library Fundamentals TS through proposal P0052R0

template <typename EF>
scope_exit<see below> make_scope_exit(EF &&exit_function) noexcept;

template <typename EF>
scope_exit<see below> make_scope_fail(EF && exit_function) noexcept;

template <typename EF>
scope_exit<see below> make_scope_success(EF && exit_function) noexcept;

On first glance this has the same caveat as std::async because you have to store the return value or the destructor will be called immediately and it won't work as expected.

ens · Answer 8 · 2018-02-01T12:47:13.563

Without commitment tracking, but extremely neat and fast.

template <typename F>
struct ScopeExit {
    ScopeExit(F&& f) : m_f(std::forward<F>(f)) {}
    ~ScopeExit() { m_f(); }
    F m_f;
};

template <typename F>
ScopeExit<F> makeScopeExit(F&& f) {
    return ScopeExit<F>(std::forward<F>(f));
};

#define STRING_JOIN(arg1, arg2) STRING_JOIN2(arg1, arg2)
#define STRING_JOIN2(arg1, arg2) arg1 ## arg2

#define ON_SCOPE_EXIT(code) auto STRING_JOIN(scopeExit, __LINE__) = makeScopeExit([&](){code;})

Usage

{
    puts("a");
    auto _ = makeScopeExit([]() { puts("b"); });
    // More readable with a macro
    ON_SCOPE_EXIT(puts("c"));
} # prints a, c, b

score 4 · Answer 9 · edited Apr 22 '12 at 22:40

4

makeScopeGuard returns a const reference. You can't store this const reference in a const ref at the caller's side in a line like:

const auto& a = RAII::makeScopeGuard( [&]() { myVec.pop_back(); } );

So you are invoking undefined behaviour.

Herb Sutter GOTW 88 gives some background about storing values in const references.

edited Apr 22 '12 at 22:40

Ken Bloom

57,498
14
111
168

answered Apr 22 '12 at 18:26

mirk

5,302
3
32
49

mirk, precisely that blog post says that is allowed, is not undefined behavior: "Normally, a temporary object lasts only until the end of the full expression in which it appears. However, C++ deliberately specifies that binding a temporary object to a reference to const on the stack lengthens the lifetime of the temporary to the lifetime of the reference itself, and thus avoids what would otherwise be a common dangling-reference error." – lurscher Apr 22 '12 at 18:42
2

@lurscher the full expression in which it appears is in the `return` statement. The temporary is destroyed when the function ends. – R. Martinho Fernandes Apr 22 '12 at 18:44
3

@lurscher Indeed. Please notice however, that in the blog the functions return values. In the code in the question above, references are returned by the functions. – mirk Apr 22 '12 at 18:45
@mirk, yeah. i missed the fact that the function was returning by value in Sutter example, and my was returning by reference, thanks! – lurscher Apr 22 '12 at 18:55
To suppress warning `unused variable 'a' [-Wunused-variable]` use `[[gnu::unused]]` attribute: `[[gnu::unused]] auto const & a = ...`. – Tomilov Anatoliy Jul 31 '15 at 11:40

maddouri · Answer 10 · 2018-10-12T21:39:09.613

FWIW I think that Andrei Alexandrescu has used a pretty neat syntax in his CppCon 2015's talk about "Declarative Control Flow" (video, slides).

The following code is heavily inspired by it:

Try It Online GitHub Gist

#include <iostream>
#include <type_traits>
#include <utility>

using std::cout;
using std::endl;

template <typename F>
struct ScopeExitGuard
{
public:
    struct Init
    {
        template <typename G>
        ScopeExitGuard<typename std::remove_reference<G>::type>
        operator+(G&& onScopeExit_)
        {
            return {false, std::forward<G>(onScopeExit_)};
        }
    };

private:
    bool m_callOnScopeExit = false;
    mutable F m_onScopeExit;

public:
    ScopeExitGuard() = delete;
    template <typename G> ScopeExitGuard(const ScopeExitGuard<G>&) = delete;
    template <typename G> void operator=(const ScopeExitGuard<G>&) = delete;
    template <typename G> void operator=(ScopeExitGuard<G>&&) = delete;

    ScopeExitGuard(const bool callOnScopeExit_, F&& onScopeExit_)
    : m_callOnScopeExit(callOnScopeExit_)
    , m_onScopeExit(std::forward<F>(onScopeExit_))
    {}

    template <typename G>
    ScopeExitGuard(ScopeExitGuard<G>&& other)
    : m_callOnScopeExit(true)
    , m_onScopeExit(std::move(other.m_onScopeExit))
    {
        other.m_callOnScopeExit = false;
    }

    ~ScopeExitGuard()
    {
        if (m_callOnScopeExit)
        {
            m_onScopeExit();
        }
    }
};

#define ON_SCOPE_EXIT_GUARD_VAR_2(line_num) _scope_exit_guard_ ## line_num ## _
#define ON_SCOPE_EXIT_GUARD_VAR(line_num) ON_SCOPE_EXIT_GUARD_VAR_2(line_num)
// usage
//     ON_SCOPE_EXIT <callable>
//
// example
//     ON_SCOPE_EXIT [] { cout << "bye" << endl; };
#define ON_SCOPE_EXIT                             \
    const auto ON_SCOPE_EXIT_GUARD_VAR(__LINE__)  \
        = ScopeExitGuard<void*>::Init{} + /* the trailing '+' is the trick to the call syntax ;) */


int main()
{
    ON_SCOPE_EXIT [] {
        cout << "on scope exit 1" << endl;
    };

    ON_SCOPE_EXIT [] {
        cout << "on scope exit 2" << endl;
    };

    cout << "in scope" << endl;  // "in scope"
}
// "on scope exit 2"
// "on scope exit 1"

For your usecase, you might also be interested in std::uncaught_exception() and std::uncaught_exceptions() to know whether your exiting the scope "normally" or after an exception has been thrown:

ON_SCOPE_EXIT [] {
    if (std::uncaught_exception()) {
        cout << "an exception has been thrown" << endl;
    }
    else {
        cout << "we're probably ok" << endl;
    }
};

HTH

thanks. As you can see, this question precedes Alexandrescu presentation by 3 years, so we had figured it out by then :-) — lurscher, Oct 13 '18 at 12:39

C.M. · Answer 11 · 2022-02-21T16:59:04.507

Here is one I came up with in C++17. It is trivial to port it to C++11 and/or add deactivation option:

template<class F>
struct scope_guard
{
    F f_;
    ~scope_guard() { f_(); }
};

template<class F> scope_guard(F) -> scope_guard<F>;

Usage:

void foo()
{
    scope_guard sg1{ []{...} };
    auto sg2 = scope_guard{ []{...} };
}

Edit: In same key here is the guard that goes off only "on exception":

#include <exception>

template<class F>
struct xguard
{
    F   f_;
    int count_ = std::uncaught_exceptions();

    ~xguard() { if (std::uncaught_exceptions() != count_) f_(); }
};

template<class F> xguard(F) -> xguard<F>;

Usage:

void foobar()
{
    xguard xg{ []{...} };
    ...
    // no need to deactivate if everything is good

    xguard{ []{...} },   // will go off only if foo() or bar() throw
        foo(),
        bar();

    // 2nd guard is no longer alive here
}

plus one for showing me `std::uncaught_exceptions`. Cool device I wasn't aware of — lurscher, Feb 18 '22 at 02:50
@lurscher also note that this solution is the most efficient -- no copy/move at all, works with function pointers too. If only C++ had two dtors -- for "leaving normally" and "unwind" cases... :) — C.M., Feb 18 '22 at 04:14

Tarc · Answer 12 · 2021-11-01T08:28:52.370

1

Here's another one, now a variation on @kwarnke's:

std::vector< int > v{ };

v.push_back( 42 );

std::shared_ptr< void > guard( nullptr , [ & v ] ( auto )
{
    v.pop_back( );
} );

edited Nov 01 '21 at 08:28

answered May 15 '17 at 21:18

Tarc

3,214
3
29
41

score 0 · Answer 13 · answered Nov 11 '13 at 13:28

You already chosen an answer, but I'll take the challenge anyway:

#include <iostream>
#include <type_traits>
#include <utility>

template < typename RollbackLambda >
class ScopeGuard;

template < typename RollbackLambda >
auto  make_ScopeGuard( RollbackLambda &&r ) -> ScopeGuard<typename
 std::decay<RollbackLambda>::type>;

template < typename RollbackLambda >
class ScopeGuard
{
    // The input may have any of: cv-qualifiers, l-value reference, or both;
    // so I don't do an exact template match.  I want the return to be just
    // "ScopeGuard," but I can't figure it out right now, so I'll make every
    // version a friend.
    template < typename AnyRollbackLambda >
    friend
    auto make_ScopeGuard( AnyRollbackLambda && ) -> ScopeGuard<typename
     std::decay<AnyRollbackLambda>::type>;

public:
    using lambda_type = RollbackLambda;

private:
    // Keep the lambda, of course, and if you really need it at the end
    bool        committed;
    lambda_type  rollback;

    // Keep the main constructor private so regular creation goes through the
    // external function.
    explicit  ScopeGuard( lambda_type rollback_action )
        : committed{ false }, rollback{ std::move(rollback_action) }
    {}

public:
    // Do allow moves
    ScopeGuard( ScopeGuard &&that )
        : committed{ that.committed }, rollback{ std::move(that.rollback) }
    { that.committed = true; }
    ScopeGuard( ScopeGuard const & ) = delete;

    // Cancel the roll-back from being called.
    void  commit()  { committed = true; }

    // The magic happens in the destructor.
    // (Too bad that there's still no way, AFAIK, to reliably check if you're
    // already in exception-caused stack unwinding.  For now, we just hope the
    // roll-back doesn't throw.)
    ~ScopeGuard()  { if (not committed) rollback(); }
};

template < typename RollbackLambda >
auto  make_ScopeGuard( RollbackLambda &&r ) -> ScopeGuard<typename
 std::decay<RollbackLambda>::type>
{
    using std::forward;

    return ScopeGuard<typename std::decay<RollbackLambda>::type>{
     forward<RollbackLambda>(r) };
}

template < typename ActionLambda, typename RollbackLambda >
auto  make_ScopeGuard( ActionLambda && a, RollbackLambda &&r, bool
 roll_back_if_action_throws ) -> ScopeGuard<typename
 std::decay<RollbackLambda>::type>
{
    using std::forward;

    if ( not roll_back_if_action_throws )  forward<ActionLambda>(a)();
    auto  result = make_ScopeGuard( forward<RollbackLambda>(r) );
    if ( roll_back_if_action_throws )  forward<ActionLambda>(a)();
    return result;
}

int  main()
{
    auto aa = make_ScopeGuard( []{std::cout << "Woah" << '\n';} );
    int  b = 1;

    try {
     auto bb = make_ScopeGuard( [&]{b *= 2; throw b;}, [&]{b = 0;}, true );
    } catch (...) {}
    std::cout << b++ << '\n';
    try {
     auto bb = make_ScopeGuard( [&]{b *= 2; throw b;}, [&]{b = 0;}, false );
    } catch (...) {}
    std::cout << b++ << '\n';

    return 0;
}
// Should write: "0", "2", and "Woah" in that order on separate lines.

Instead of having creation functions and a constructor, you limited to just the creation functions, with the main constructor being private. I couldn't figure out how to limit the friend-ed instantiations to just the ones involving the current template parameter. (Maybe because the parameter is mentioned only in the return type.) Maybe a fix to that can be asked on this site. Since the first action doesn't need to be stored, it's present only in the creation functions. There's a Boolean parameter to flag if throwing from the first action triggers a roll-back or not.

The std::decay part strips both cv-qualifiers and reference markers. But you can't use it for this general purpose if the input type is a built-in array, since it'll apply the array-to-pointer conversion too.

ricab · Answer 14 · 2018-08-15T22:15:32.933

0

Yet another answer, but I am afraid I find the others all lacking in one way or another. Notably, the accepted answer dates from 2012, but it has an important bug (see this comment). This shows the importance of testing.

Here is an implementation of a >=C++11 scope_guard that is openly available and extensively tested. It is meant to be/have:

modern, elegant, simple (mostly single-function interface and no macros)
general (accepts any callable that respects preconditions)
carefully documented
thin callback wrapping (no added std::function or virtual table penalties)
proper exception specifications

The simplest and neatest c++11 ScopeGuard

14 Answers14

Linked

Related