In C++ is there an idiomatic way to guard against the situation in which running a collection of actions causes the collection to be mutated?

Question

Say you have a class foo that wraps a collection of some kind of callable objects. foo has a member function run() that iterates over the collection and calls each function object. foo also has a member remove(...) that will remove a callable object from the collection.

Is there an idiomatic, RAII-style guard you can put in foo.run() and foo.remove(...) such that the removes that were driven by a call to foo.run() will be deferred until the guard's destructor fires? Can it be done with something in the standard library? Does this pattern have a name?

My current code seems inelegant so am looking for a best practice type solution.

Note: this is isnt about concurrency. A non thread-safe solution is fine. The issue is with re-entrancy and self-reference.

Here is an example of the problem, sans the inelegant "defer remove" guard.

class ActionPlayer
{
private:
    std::vector<std::pair<int, std::function<void()>>> actions_;
public:
    void addAction(int id, const std::function<void()>& action)
    {
        actions_.push_back({ id, action });
    }

    void removeAction(int id)
    {
        actions_.erase(
            std::remove_if(
                actions_.begin(),
                actions_.end(),
                [id](auto& p) { return p.first == id; }
            ),
            actions_.end()
        );
    }

    void run()
    {
        for (auto& item : actions_) {
            item.second();
        }
    }
};

then elsewhere:

...

ActionPlayer player;

player.addAction(1, []() {
    std::cout << "Hello there" << std::endl;
});

player.addAction(42, [&player]() {
    std::cout << "foobar" << std::endl;
    player.removeAction(1);
});

player.run(); // boom

Edit ... okay this is how I can see to do this via an RAII lock object. The following should handle actions throwing and re-entrant calls to run within run assuming the recursion eventually terminates (if not that is the user's fault). I used cached std::functions because in the real version of this code the equivalent of addAction and removeAction are template functions which cant just be stored in a vanilla homogeneously typed container.

class ActionPlayer
{
private:

    std::vector<std::pair<int, std::function<void()>>> actions_;
    int run_lock_count_;
    std::vector<std::function<void()>> deferred_ops_;

    class RunLock
    {
    private:
        ActionPlayer* parent_;
    public:
        RunLock(ActionPlayer* parent) : parent_(parent) { (parent_->run_lock_count_)++; }
        ~RunLock()
        {
            if (--parent_->run_lock_count_ == 0) {
                while (!parent_->deferred_ops_.empty()) {
                    auto do_deferred_op = parent_->deferred_ops_.back();
                    parent_->deferred_ops_.pop_back();
                    do_deferred_op();
                }
            }
        }
    };

    bool isFiring() const
    {
        return run_lock_count_ > 0;
    }

public:
    ActionPlayer() : run_lock_count_(0)
    {
    }

    void addAction(int id, const std::function<void()>& action)
    {
        if (!isFiring()) {
            actions_.push_back({ id, action });
        } else {
            deferred_ops_.push_back(
                [&]() {
                    addAction(id, action);
                }
            );
        }
    }

    void removeAction(int id)
    {
        if (!isFiring()) {
            actions_.erase(
                std::remove_if(
                    actions_.begin(),
                    actions_.end(),
                    [id](auto& p) { return p.first == id; }
                ),
                actions_.end()
            );
        } else {
            deferred_ops_.push_back(
                [&]() {
                    removeAction(id); 
                }
            );
        }
    }

    void run()
    {
        RunLock lock(this);
        for (auto& item : actions_) {
            item.second();
        }
    }
};

Answer 1

Add a flag to run that says you're enumerating thru actions_. Then if removeAction is called with that flag set, you store the id in a vector for later removal. You might also need a separate vector to hold actions that are added while enumerating. Once you're all done iterating thru actions_, you remove the ones that want removed and add the ones that got added.

Something like

// within class ActionPlayer, add these private member variables
private:
    bool running = false;
    std::vector<int> idsToDelete;

public:
    void run() {
        running = true;
        for (auto& item : actions_) {
            item.second();
        }
        running = false;
        for (d: idsToDelete)
            removeAction(d);
        idsToDelete.clear();
    }

    // ...

You can make a similar change for deferred addAction calls (which you'll need to do if any of the actions can add an action, since the add might cause the vector to allocate more storage, invalidating all iterators to the vector).