The simplest and neatest c++11 ScopeGuard
Asked Answered
T

14

49

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();
            }
        };
    }
}
Titanesque answered 22/4, 2012 at 17:35 Comment(16)
One thing you're leaving out is that this code doesn't quite compile. The declarations of the guard variables are missing template parameters.Gobi
@R.MartinhoFernandes shouldn't the compiler be able to deduce from the arguments? these are lambdas, so as you know their type is opaqueTitanesque
right i'm missing the make_scopeGuard, give me a minuteTitanesque
@R.MartinhoFernandes so i keep the temporaries in const references, and changed the commited member to mutable so its modifiable in a const objectTitanesque
@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.Gobi
@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?Paraphrase
@Griwes, because a reference to a temporary can only be held with const references. This is explained in the original Alexandrescu article about ScopeGuardTitanesque
@Titanesque Or with an rvalue reference.Gobi
@lurscher, eh? Where do you have references in commit()?Paraphrase
@R.MartinhoFernandes i'm sorry, it is working nowTitanesque
@Griwes, the references are a and b and they reference two temporary objectsTitanesque
@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; }?Paraphrase
@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 methodTitanesque
@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 complexityTitanesque
I just posted a much simpler version based on the same article. Note that I don't need generator functions.Danaides
I didn't know that this idiom is called Scoped Guard. So I created a similar question: https://mcmap.net/q/24814/-how-do-i-run-a-cleanup-code-on-the-function-exit/5447906 (solutions from there may be helpful).Gunpowder
G
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 https://mcmap.net/q/24815/-is-substitution-failure-an-error-with-dependent-non-type-template-parameters/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 ));
}
Gobi answered 22/4, 2012 at 18:27 Comment(12)
R. Martinho, read the Herb Sutter blog posted by mirk, const references to temporaries is not undefined behaviorTitanesque
@Titanesque No, you go back and read it carefully. The functions there return by value.Gobi
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 thatTitanesque
Exactly. The reference is immediately dangling the moment it's returned.Gobi
quick question: why you had to add std::forward<AdquireLambda>(_al)();, whats the difference between that and simply doing _al(); inside the move constructor?Titanesque
@Titanesque just in case you use a function object like this: struct foo { void operator()() && /* not the rvalue qualifier */; };.Gobi
what is that rvalue qualifier? shouldn't that qualifier be in the return type? i haven't seen that usage of && beforeTitanesque
@Titanesque There's a very comprehensive explanation here: https://mcmap.net/q/24017/-what-is-the-quot-rvalue-reference-for-this-quot-proposal. It's not a feature you'll use everyday, but when writing generic code, it's good to be as generic as possible. :)Gobi
Couldn't you significantly simplify this by using non-template class and storing the cleanup function in std::function?Sarpedon
@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.Calen
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.Elaterite
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 answerHelaina
D
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(); };

// ...
Danaides answered 9/2, 2015 at 15:34 Comment(27)
"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!Mahler
The best so far, IMHOTeresita
Do note that if the captured variables don't fit within the in-place buffer of std::function, it will incur dynamic allocation.Transliterate
@Transliterate If this is a concern you could use function pointers, e.g. you could write scope_guard guard1 = revert1;.Danaides
@Danaides 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.Transliterate
Also, std::function<Fn> uses a level of indirection and thus has more overhead.Outdated
throw() is deprecated in C++11, noexcept replaces it.Unipolar
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.Wendolyn
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.Wendolyn
@Wendolyn 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.Danaides
@Wendolyn 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.Danaides
Wouldn't private inheritance be better in this scenario?Jensen
@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.Danaides
@Danaides 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_guards, which wasn't possible with templated definitions.Jensen
std::function constructor may throw. If it throws, the cleanup code won't be executed.Gunpowder
@Gunpowder 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.Danaides
@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://mcmap.net/q/24814/-how-do-i-run-a-cleanup-code-on-the-function-exit), but it has more complex implementation.Gunpowder
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.Danaides
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?Plains
@davidfong Works for me... ideone.com/hmuN8u scope_guard b = std::move(a);Danaides
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.Contrastive
@AlexW In what situation would the move succeed (or the moved callable invalidated) and an exception thrown?Danaides
@Danaides The move constructor of std::function is not marked noexcept for C++11, C++14, and C++17. Additionally, cpp-reference 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.Contrastive
@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.Danaides
@Danaides 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).Contrastive
@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.Danaides
@Danaides 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!Contrastive
G
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 https://mcmap.net/q/24815/-is-substitution-failure-an-error-with-dependent-non-type-template-parameters/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 ));
}
Gobi answered 22/4, 2012 at 18:27 Comment(12)
R. Martinho, read the Herb Sutter blog posted by mirk, const references to temporaries is not undefined behaviorTitanesque
@Titanesque No, you go back and read it carefully. The functions there return by value.Gobi
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 thatTitanesque
Exactly. The reference is immediately dangling the moment it's returned.Gobi
quick question: why you had to add std::forward<AdquireLambda>(_al)();, whats the difference between that and simply doing _al(); inside the move constructor?Titanesque
@Titanesque just in case you use a function object like this: struct foo { void operator()() && /* not the rvalue qualifier */; };.Gobi
what is that rvalue qualifier? shouldn't that qualifier be in the return type? i haven't seen that usage of && beforeTitanesque
@Titanesque There's a very comprehensive explanation here: https://mcmap.net/q/24017/-what-is-the-quot-rvalue-reference-for-this-quot-proposal. It's not a feature you'll use everyday, but when writing generic code, it's good to be as generic as possible. :)Gobi
Couldn't you significantly simplify this by using non-template class and storing the cleanup function in std::function?Sarpedon
@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.Calen
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.Elaterite
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 answerHelaina
S
19

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

shared_ptr<int> x(NULL, [&](int *) { CloseResource(); });
Suzy answered 8/5, 2018 at 18:48 Comment(18)
...alas, it allocates memory :-\Woolworth
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.Suzy
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).Woolworth
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.Suzy
And if you were really worried about memory allocation and wanted a lot of control of your program, you'd use C not C++Suzy
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.Woolworth
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.Suzy
lol... kinda true, depending on where you work. There are still places where (almost) correct code is still valued. Almost because this makes it impossible to write correct code (that uses exceptions), have to fallback on external solutions (auto-restart, microservices, etc)Woolworth
and that's why you don't use exceptions. :-) c++ is just as broken as all the other broken software. :-)Suzy
Nah... I like mechanism of exceptions -- it just needs some fixin'. C++ isn't really broken, but messy -- there is no denying that. :)Woolworth
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.Suzy
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.Woolworth
...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)Woolworth
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.Suzy
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.Woolworth
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. :-)Suzy
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.Flinders
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.Suzy
S
15

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

Sharpen answered 8/3, 2013 at 22:29 Comment(3)
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?Duwalt
Yes summary needed - the link is a 70 minute long video presentation.Unipolar
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, which includes a ScopeGuard implementation. It's worth a look.Chacha
C
15

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.

Cesarean answered 17/6, 2016 at 10:35 Comment(0)
M
12

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.

Mahler answered 16/4, 2020 at 4:29 Comment(5)
Sorry to answer your 4 year old question. But could you explain what `template<typename F> OnExit(F&& frv) -> OnExit<F>;``actually does ? Or where I find what actually happens there ? Or give me references where I can read it up?Perspiration
@Perspiration look up "deduction guide" (I explain this in the last sentence)Mahler
I like this solution, but what is the advantage of not moving the lambda...?Gibbosity
@jb if the lambda has captures then moving it might be expensive or incorrectMahler
This guy f**s!!Mongolic
S
8

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.

Sollars answered 24/1, 2016 at 13:42 Comment(0)
U
5

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
Unexperienced answered 28/2, 2017 at 10:53 Comment(0)
S
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.

Sara answered 22/4, 2012 at 18:26 Comment(5)
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."Titanesque
@Titanesque the full expression in which it appears is in the return statement. The temporary is destroyed when the function ends.Gobi
@Titanesque 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.Sara
@mirk, yeah. i missed the fact that the function was returning by value in Sutter example, and my was returning by reference, thanks!Titanesque
To suppress warning unused variable 'a' [-Wunused-variable] use [[gnu::unused]] attribute: [[gnu::unused]] auto const & a = ....Laverne
S
3

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

Stavropol answered 12/10, 2018 at 21:25 Comment(1)
thanks. As you can see, this question precedes Alexandrescu presentation by 3 years, so we had figured it out by then :-)Titanesque
W
3

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
}
Woolworth answered 16/2, 2022 at 22:7 Comment(2)
plus one for showing me std::uncaught_exceptions. Cool device I wasn't aware ofTitanesque
@Titanesque 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... :)Woolworth
V
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( );
} );
Vespertine answered 15/5, 2017 at 21:18 Comment(0)
C
0

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.

Ceylon answered 11/11, 2013 at 13:28 Comment(0)
H
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

See also the full list of features.

Helaina answered 15/8, 2018 at 21:51 Comment(3)
Does it have advantages over Boost.ScopeExit?Covey
@MaxBarraclough It is targeted at >= C++11, so no macros or capture tricks (leaving that to lambdas); it protects against forgotten returns; has an option to enforce noexcept in C++17; has modern exception specifications (noexcept); is sfinae friendly; single "drag and drop" header; unlicensed.Helaina
Thanks. What do you mean by protecting against forgotten returns? That's UB no matter what, no?Covey

© 2022 - 2024 — McMap. All rights reserved.